Skip to content

Commit f98c564

Browse files
LorenVSLoren Van Spronsen
andauthored
Address infinite loops while reading ID3v2 tags. (#48)
The `Buffer.read()` call will infinitely loop if the number of bytes remaining in the file is less than the requested size. A large number of `read()` calls in `ID3v2Parser` are at risk of infinitely looping on malformed files. I'm changing `Buffer` to throw an exception when it detects there are no more bytes to read from the file. There is a more egregious case with the potential `Xing` header. The parser blindly reads 1500 bytes to search for the header, but there is no guarantee (even in a well-formed file) that there will be 1500 more bytes remaining to be read. I'm introducing a `readAtMost` method to `Buffer` which will handle cases where there are fewer than `size` bytes remaining in the file. I'm also updating some of the logic which is checking for the start of the `Xing` header to be safer, checking to ensure that there are enough bytes remaining to parse the data from this header. Co-authored-by: Loren Van Spronsen <lorenvs@google.com>
1 parent 68b0915 commit f98c564

File tree

4 files changed

+52
-4
lines changed

4 files changed

+52
-4
lines changed

lib/src/parsers/id3v2.dart

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -246,15 +246,16 @@ class ID3v2Parser extends TagParser {
246246
metadata.bitrate = _getBitrate(mpegVersion, mpegLayer, bitrateIndex);
247247

248248
// arbitrary choice. Usually the `Xing` header is located after ~30 bytes
249-
// then the header size is about ~150 bytes
250-
final possibleXingHeader = buffer.read(1500);
249+
// then the header size is about ~150 bytes.
250+
final possibleXingHeader = buffer.readAtMost(1500);
251251

252252
int i = 0;
253-
while (possibleXingHeader[i] == 0) {
253+
while (i < possibleXingHeader.length && possibleXingHeader[i] == 0) {
254254
i++;
255255
}
256256

257-
if (possibleXingHeader[i] == 0x58 &&
257+
if ((i < possibleXingHeader.length - 11) &&
258+
possibleXingHeader[i] == 0x58 &&
258259
possibleXingHeader[i + 1] == 0X69 &&
259260
possibleXingHeader[i + 2] == 0x6E &&
260261
possibleXingHeader[i + 3] == 0x67) {

lib/src/utils/buffer.dart

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,21 @@
11
import 'dart:io';
2+
import 'dart:math';
23
import 'dart:typed_data';
34

5+
import '../../audio_metadata_reader.dart';
6+
47
class Buffer {
58
final RandomAccessFile randomAccessFile;
69
final Uint8List _buffer;
710
int _cursor = 0;
811
int _bufferedBytes = 0; // Track how many bytes are actually in the buffer
912
static final int _bufferSize = 16384;
1013

14+
/// The number of bytes remaining to be read from the file.
15+
int get remainingBytes =>
16+
(_bufferedBytes - _cursor) +
17+
(randomAccessFile.lengthSync() - randomAccessFile.positionSync());
18+
1119
Buffer({required this.randomAccessFile}) : _buffer = Uint8List(_bufferSize) {
1220
_fill();
1321
}
@@ -17,6 +25,19 @@ class Buffer {
1725
_cursor = 0;
1826
}
1927

28+
/// Throws a [MetadataParserException] if a previous call to [_fill]
29+
/// was unable to read any data from the file.
30+
///
31+
/// Once the end of the file is reached, subsequent reads from
32+
/// [RandomAccessFile] will read 0 bytes without failing. This
33+
/// can cause [read] below to infinite loop.
34+
void _throwOnNoData() {
35+
if (_bufferedBytes == 0) {
36+
throw MetadataParserException(
37+
track: File(""), message: "Expected more data in file");
38+
}
39+
}
40+
2041
Uint8List read(int size) {
2142
// if we read something big (~100kb), we can read it directly from file
2243
// it makes the read faster
@@ -61,12 +82,24 @@ class Buffer {
6182
// Fill buffer again if more data is needed
6283
if (filled < size) {
6384
_fill();
85+
// Avoid infinite loops if we are trying to read
86+
// more data than there is left in the file.
87+
_throwOnNoData();
6488
}
6589
}
6690
return result;
6791
}
6892
}
6993

94+
/// Reads at most [size] bytes from the file.
95+
///
96+
/// May return a smaller list if [remainingBytes] is
97+
/// less than [size].
98+
Uint8List readAtMost(int size) {
99+
final readSize = min(size, remainingBytes);
100+
return read(readSize);
101+
}
102+
70103
void setPositionSync(int position) {
71104
randomAccessFile.setPositionSync(position);
72105
_fill();
205 Bytes
Binary file not shown.

test/mp3/mp3_parser_test.dart

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,18 @@ void main() {
4646
expect(result.duration, Duration(minutes: 3, seconds: 22));
4747
expect(result.sampleRate, 44100);
4848
});
49+
50+
test("Parses from truncated mp3 file", () {
51+
// The caress-your-soul-truncated-cleaned.mp3 file is truncated
52+
// immediately before the Xing header. We should still be able
53+
// to read the ID3 tag data.
54+
final track = File("./test/mp3/caress-your-soul-cleaned-truncated.mp3");
55+
final result = readMetadata(track, getImage: false);
56+
expect(result.pictures.length, 0);
57+
expect(result.album, "Caress Your Soul");
58+
expect(result.title, "How to Fly");
59+
expect(result.artist, "Sticky Fingers");
60+
expect(result.year, DateTime(2013));
61+
expect(result.sampleRate, 44100);
62+
});
4963
}

0 commit comments

Comments
 (0)