-
Notifications
You must be signed in to change notification settings - Fork 188
Description
Hello. I was using ImageMetadataReader.ReadMetadata(stream) to read the metadata from an image and got an System.OverflowException.
I did some digging and found out that this is due to an XMP segment only containg the preamble bytes.
Let's start with a test for reproduction:
var bytes = new byte[] {
0xFF, 0xD8, // MagicNumber for JPEG format
// Start of original segment
0xFF, // SegmentIdentifier
0xE1, // segmentTypeByte JpegSegmentType.App1 = 0xE1
0x00, 0x1F, // SegmentLength <high-byte> <low-byte> equal to 31 but this includes 2 bytes for the size byte so actually 29
// The following is the byte-equivalent of Encoding.UTF8.GetBytes("http://ns.adobe.com/xap/1.0/\0")
0x68, 0x74, 0x74, 0x70, 0x3A, 0x2F, 0x2F, 0x6E, 0x73, 0x2E, 0x61, 0x64, 0x6F, 0x62, 0x65, 0x2E, 0x63, 0x6F, 0x6D, 0x2F, 0x78, 0x61, 0x70, 0x2F, 0x31, 0x2E, 0x30, 0x2F, 0x00,
// End of original segment
0xFF, 0xDA, 0x00, 0x00 // Start of Scan segment to get reader to break out of loop
};
using var stream = new MemoryStream(bytes);
var jpegSegments = JpegSegmentReader.ReadSegments(new SequentialStreamReader(stream), [JpegSegmentType.App1]).ToList();
Assert.That(jpegSegments, Has.Count.EqualTo(1));
var segment = jpegSegments.Single();
Assert.That(segment.Type, Is.EqualTo(JpegSegmentType.App1));
Assert.That(segment.Bytes, Is.EqualTo(Encoding.UTF8.GetBytes("http://ns.adobe.com/xap/1.0/\0")));
var xmpReader = new XmpReader();
Assert.DoesNotThrow(() => xmpReader.ReadJpegSegments(jpegSegments).ToList());which results in triggering the OverflowException.
Above code causes the call Extract(Encoding.UTF8.GetBytes(xmpBytes: "http://ns.adobe.com/xap/1.0/\0"), offset: 29, length: 0) which is fine on its own.
However Extract contains the following code
// Trim any trailing null bytes
// https://github.com/drewnoakes/metadata-extractor-dotnet/issues/154
while (xmpBytes[offset + length - 1] == 0)
length--;which is true for one iteration as the preambly ends with 0x00 causing length to become -1. This causes new ByteBuffer(buffer, offset, length) inside the later called XmpMetaFactory.ParseFromBuffer to fail with the mentioned OverflowException.
My guess is that this could be fixed by simply having a check for length > 0 inside the mentioned loop condition, but I don't know all the cases well enough to know if this is true. Any input is appreciated