Closed Bug 1468539 Opened 4 years ago Closed 4 years ago
S Due to Negative fseek()
47 bytes, text/x-phabricator-request
|Details | Review|
mar_read_product_info_block() retrieved the variable numAdditionalBlocks via get_mar_file_info_fp(). The value was read from the file without further sanitization. The function then iterated over the file numAdditionalBlocks times to read each block, stopping when a read failed. After the size of a block was read, fseek() was used to skip over this block. But since the fseek() operated on a value retrieved from the file which was not further checked, it could seek in a negative direction, causing the loop to read the same block over and over again. This might cause a limited DoS issue, since the loop is bounded to numAdditionalBlocks iterations. It is advised to add sanity checks to additionalBlockSize and numAdditionalBlocks.
Assignee: nobody → jewilde
Status: NEW → ASSIGNED
Priority: -- → P1
Only a single type of additional block has ever been defined for the MAR archive format and only a single block of that type is needed per file so limiting ourselves to reading only that until we define more seems sensible
Only a single type of additional block has ever been defined for the MAR archive format and only a single block of that type is needed per file. Limiting ourselves to reading only that until we define more seems sensible. Move additionalBlockSize check before first fread Add MAXADDITIONALBLOCKSIZE as a constant for checking block sizes
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main65-]
You need to log in before you can comment on or make changes to this bug.