Skip to content

Commit 820746d

Browse files
committed
Fixes #51
Corrected stream bounds checks in SubStream.Read, BlockCacheStream.Read, LinuxRaidVolume.GetSuperblock, UdfReader.Detect and HfsPlus.FileBuffer.Read. This should avoid some unnecessary and unexpected exceptions in places where the error can be handles by calling code in a better way.
1 parent 17059e1 commit 820746d

File tree

5 files changed

+77
-59
lines changed

5 files changed

+77
-59
lines changed

Library/DiscUtils.HfsPlus/FileBuffer.cs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,19 @@ public override int Read(long pos, byte[] buffer, int offset, int count)
105105

106106
var volStream = _context.RawStream;
107107
volStream.Position = extentStreamStart + extentOffset;
108+
109+
if (volStream.Position + toRead > volStream.Length)
110+
{
111+
throw new NotSupportedException("Unsupported HFS+ file system: Cannot find allocation for file");
112+
}
113+
108114
var numRead = volStream.Read(buffer, offset + totalRead, toRead);
109115

116+
if (numRead < toRead)
117+
{
118+
throw new NotSupportedException("Unsupported HFS+ file system: Cannot find allocation for file");
119+
}
120+
110121
totalRead += numRead;
111122
}
112123

@@ -137,8 +148,19 @@ public override async ValueTask<int> ReadAsync(long pos, Memory<byte> buffer, Ca
137148

138149
var volStream = _context.RawStream;
139150
volStream.Position = extentStreamStart + extentOffset;
151+
152+
if (volStream.Position + toRead > volStream.Length)
153+
{
154+
throw new NotSupportedException("Unsupported HFS+ file system: Cannot find allocation for file");
155+
}
156+
140157
var numRead = await volStream.ReadAsync(buffer.Slice(totalRead, toRead), cancellationToken).ConfigureAwait(false);
141158

159+
if (numRead < toRead)
160+
{
161+
throw new NotSupportedException("Unsupported HFS+ file system: Cannot find allocation for file");
162+
}
163+
142164
totalRead += numRead;
143165
}
144166

@@ -169,8 +191,19 @@ public override int Read(long pos, Span<byte> buffer)
169191

170192
var volStream = _context.RawStream;
171193
volStream.Position = extentStreamStart + extentOffset;
194+
195+
if (volStream.Position + toRead > volStream.Length)
196+
{
197+
throw new NotSupportedException("Unsupported HFS+ file system: Cannot find allocation for file");
198+
}
199+
172200
var numRead = volStream.Read(buffer.Slice(totalRead, toRead));
173201

202+
if (numRead < toRead)
203+
{
204+
throw new NotSupportedException("Unsupported HFS+ file system: Cannot find allocation for file");
205+
}
206+
174207
totalRead += numRead;
175208
}
176209

Library/DiscUtils.Lvm/LinuxRaid/LinuxRaidDiskVolume.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,13 @@ internal static LinuxRaidSuperblock GetSuperblock(PartitionInfo partition)
9191
{
9292
try
9393
{
94+
if (offset > volumeStream.Length)
95+
{
96+
continue;
97+
}
98+
9499
volumeStream.Position = offset;
100+
95101
var bytesRead = volumeStream.Read(buffer);
96102
if (bytesRead >= 512) // Minimum superblock size
97103
{

Library/DiscUtils.Streams/Block/BlockCacheStream.cs

Lines changed: 15 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ namespace DiscUtils.Streams;
3333
/// </summary>
3434
public sealed class BlockCacheStream : SparseStream
3535
{
36-
private bool _atEof;
3736
private readonly int _blocksInReadBuffer;
3837

3938
private readonly BlockCache<Block> _cache;
@@ -186,14 +185,13 @@ public override int Read(byte[] buffer, int offset, int count)
186185
{
187186
CheckDisposed();
188187

189-
if (_position >= Length)
188+
if (_position > Length)
190189
{
191-
if (_atEof)
192-
{
193-
throw new IOException("Attempt to read beyond end of stream");
194-
}
190+
throw new IOException("Attempt to read beyond end of stream");
191+
}
195192

196-
_atEof = true;
193+
if (_position == Length)
194+
{
197195
return 0;
198196
}
199197

@@ -207,11 +205,6 @@ public override int Read(byte[] buffer, int offset, int count)
207205
var numRead = _wrappedStream.Read(buffer, offset, count);
208206
_position = _wrappedStream.Position;
209207

210-
if (_position >= Length)
211-
{
212-
_atEof = true;
213-
}
214-
215208
return numRead;
216209
}
217210

@@ -297,11 +290,6 @@ public override int Read(byte[] buffer, int offset, int count)
297290
}
298291
}
299292

300-
if (_position >= Length && totalBytesRead == 0)
301-
{
302-
_atEof = true;
303-
}
304-
305293
if (servicedFromCache)
306294
{
307295
_stats.ReadCacheHits++;
@@ -324,14 +312,13 @@ public override int Read(Span<byte> buffer)
324312
{
325313
CheckDisposed();
326314

327-
if (_position >= Length)
315+
if (_position > Length)
328316
{
329-
if (_atEof)
330-
{
331-
throw new IOException("Attempt to read beyond end of stream");
332-
}
317+
throw new IOException("Attempt to read beyond end of stream");
318+
}
333319

334-
_atEof = true;
320+
if (_position == Length)
321+
{
335322
return 0;
336323
}
337324

@@ -345,11 +332,6 @@ public override int Read(Span<byte> buffer)
345332
var numRead = _wrappedStream.Read(buffer);
346333
_position = _wrappedStream.Position;
347334

348-
if (_position >= Length)
349-
{
350-
_atEof = true;
351-
}
352-
353335
return numRead;
354336
}
355337

@@ -435,11 +417,6 @@ public override int Read(Span<byte> buffer)
435417
}
436418
}
437419

438-
if (_position >= Length && totalBytesRead == 0)
439-
{
440-
_atEof = true;
441-
}
442-
443420
if (servicedFromCache)
444421
{
445422
_stats.ReadCacheHits++;
@@ -463,14 +440,13 @@ public override async ValueTask<int> ReadAsync(Memory<byte> buffer, Cancellation
463440
{
464441
CheckDisposed();
465442

466-
if (_position >= Length)
443+
if (_position > Length)
467444
{
468-
if (_atEof)
469-
{
470-
throw new IOException("Attempt to read beyond end of stream");
471-
}
445+
throw new IOException("Attempt to read beyond end of stream");
446+
}
472447

473-
_atEof = true;
448+
if (_position == Length)
449+
{
474450
return 0;
475451
}
476452

@@ -484,11 +460,6 @@ public override async ValueTask<int> ReadAsync(Memory<byte> buffer, Cancellation
484460
var numRead = await _wrappedStream.ReadAsync(buffer, cancellationToken).ConfigureAwait(false);
485461
_position = _wrappedStream.Position;
486462

487-
if (_position >= Length)
488-
{
489-
_atEof = true;
490-
}
491-
492463
return numRead;
493464
}
494465

@@ -574,11 +545,6 @@ public override async ValueTask<int> ReadAsync(Memory<byte> buffer, Cancellation
574545
}
575546
}
576547

577-
if (_position >= Length && totalBytesRead == 0)
578-
{
579-
_atEof = true;
580-
}
581-
582548
if (servicedFromCache)
583549
{
584550
_stats.ReadCacheHits++;
@@ -621,8 +587,6 @@ public override long Seek(long offset, SeekOrigin origin)
621587
effectiveOffset += Length;
622588
}
623589

624-
_atEof = false;
625-
626590
if (effectiveOffset < 0)
627591
{
628592
throw new IOException("Attempt to move before beginning of disk");

Library/DiscUtils.Streams/SubStream.cs

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,11 +119,16 @@ public override int Read(byte[] buffer, int offset, int count)
119119
throw new ArgumentOutOfRangeException(nameof(count), "Attempt to read negative bytes");
120120
}
121121

122-
if (Position < 0 || Position > _length)
122+
if (Position < 0 || Position == _length)
123123
{
124124
return 0;
125125
}
126126

127+
if (Position > _length)
128+
{
129+
throw new EndOfStreamException("Attempt to read beyond end of substream");
130+
}
131+
127132
_parent.Position = _first + Position;
128133
var numRead = _parent.Read(buffer, offset,
129134
(int)Math.Min(count, Math.Min(_length - Position, int.MaxValue)));
@@ -133,11 +138,16 @@ public override int Read(byte[] buffer, int offset, int count)
133138

134139
public override async ValueTask<int> ReadAsync(Memory<byte> buffer, CancellationToken cancellationToken)
135140
{
136-
if (Position < 0 || Position > _length)
141+
if (Position < 0 || Position == _length)
137142
{
138143
return 0;
139144
}
140145

146+
if (Position > _length)
147+
{
148+
throw new EndOfStreamException("Attempt to read beyond end of substream");
149+
}
150+
141151
_parent.Position = _first + Position;
142152
var numRead = await _parent.ReadAsync(buffer.Slice(0, (int)Math.Min(buffer.Length, Math.Min(_length - Position, int.MaxValue))), cancellationToken).ConfigureAwait(false);
143153
Position += numRead;
@@ -146,11 +156,16 @@ public override async ValueTask<int> ReadAsync(Memory<byte> buffer, Cancellation
146156

147157
public override int Read(Span<byte> buffer)
148158
{
149-
if (Position < 0 || Position > _length)
159+
if (Position < 0 || Position == _length)
150160
{
151161
return 0;
152162
}
153163

164+
if (Position > _length)
165+
{
166+
throw new EndOfStreamException("Attempt to read beyond end of substream");
167+
}
168+
154169
_parent.Position = _first + Position;
155170
var numRead = _parent.Read(buffer.Slice(0, (int)Math.Min(buffer.Length, Math.Min(_length - Position, int.MaxValue))));
156171
Position += numRead;

Library/DiscUtils.Udf/UdfReader.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,6 @@ public UdfReader(Stream data, int sectorSize)
5656
/// <returns><c>true</c> if the stream contains a UDF file system, else false.</returns>
5757
public static bool Detect(Stream data)
5858
{
59-
if (data.Length < IsoUtilities.SectorSize)
60-
{
61-
return false;
62-
}
63-
6459
long vdpos = 0x8000; // Skip lead-in
6560

6661
Span<byte> buffer = stackalloc byte[IsoUtilities.SectorSize];
@@ -71,6 +66,11 @@ public static bool Detect(Stream data)
7166
BaseVolumeDescriptor bvd;
7267
while (validDescriptor)
7368
{
69+
if (vdpos + IsoUtilities.SectorSize > data.Length)
70+
{
71+
return false;
72+
}
73+
7474
data.Position = vdpos;
7575
var numRead = data.ReadMaximum(buffer);
7676
if (numRead != IsoUtilities.SectorSize)

0 commit comments

Comments
 (0)