engine apng: reject short fdAT chunk lengths#183180
Conversation
There was a problem hiding this comment.
Code Review
This pull request modifies the APNG image decoding logic to handle fdAT chunks with short data lengths. In APNGImageGenerator::DemuxNextImage, a check is added to ensure the data_length of an fdAT chunk is at least 4 bytes. If the length is less than 4, the function now returns an error, preventing a potential integer underflow and subsequent buffer overflow in memcpy. A new test file, image_generator_apng_unittests.cc, is added with two tests: one that verifies that an APNG with an fdAT chunk of length 0 is rejected, and another that confirms an APNG with an fdAT chunk of length 4 is accepted. The BUILD.gn file is updated to include the new test file and its associated image fixtures.
|
hey @1seal you can run |
|
This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
guard against fdAT data_length < 4 underflow in APNGImageGenerator::DemuxNextImage and add a ui_unittests regression test + minimal fixtures.
cbc9966 to
f6538cb
Compare
|
rebased and fixed the failing positive control test. apng_fdat_len4.apng was boundary-safe for the underflow check, but not a valid APNG acceptance fixture on current master, so ui_unittests failed. switched to a valid fdAT APNG and re-triggered CI. |
…ing malformed fdAT chunks (flutter#186700) An fdAT (frame data) chunk in an APNG should contain a sequence number followed by image data. The APNG decoder needs to reject invalid fdAT chunks that do not have the expected contents. Based on flutter#184301 and flutter#183180 Fixes flutter#183179 --------- Co-authored-by: 1seal <security@1seal.org> Co-authored-by: mohammadmseet-hue <mohammadmseet@gmail.com>
|
Thank you for your contribution, and sorry for taking so long to respond. I've landed a different version of the fix for this issue: #186700 |
…ing malformed fdAT chunks (flutter#186700) An fdAT (frame data) chunk in an APNG should contain a sequence number followed by image data. The APNG decoder needs to reject invalid fdAT chunks that do not have the expected contents. Based on flutter#184301 and flutter#183180 Fixes flutter#183179 --------- Co-authored-by: 1seal <security@1seal.org> Co-authored-by: mohammadmseet-hue <mohammadmseet@gmail.com>
fixes an apng demux edge case in engine: fdAT chunks with data_length < 4 (sequence_number minimum) previously underflowed (data_length - 4) and that value was used as the memcpy length during fdAT->IDAT rewrite.
changes:
issue: #183179