Added sanity check for every jpeg marker#2084
Conversation
| // to uint to avoid sign extension | ||
| if (stream.RemainingBytes < (uint)markerContentByteSize) | ||
| { | ||
| JpegThrowHelper.ThrowNotEnoughBytesForMarker(fileMarker.Marker); |
There was a problem hiding this comment.
Note: this error message would contain hex value of the marker. While we can map byte to ITU spec name I don't think it's worth the effort.
antonfirsov
left a comment
There was a problem hiding this comment.
Changes look good. Does it make sense to construct at least one faulty image with a hex editor for regression testing? (Ideally I would overflow the last - non SOS - marker in the file.)
Sounds reasonable, will create one today. |
|
@antonfirsov wow, latest release version can actually fall into really dangerous code regions for malformed jpegs.
New stacktrace: |
Not that terrible, could be access violation 😆 |
Prerequisites
Description
Discussed at #2077. Now before even trying to parse any jpeg marker decoder would check whether input stream has enough bytes available thus there's no need to check any
stream.Read(...)call for return value.Closes #2085.