Skip to content

reject out-of-range component quant_tbl_no in encoder - #256

Open
rootvector2 wants to merge 1 commit into
google:mainfrom
rootvector2:quant-tbl-no-bound
Open

rootvector2 wants to merge 1 commit into
google:mainfrom
rootvector2:quant-tbl-no-bound

Conversation

@rootvector2

Copy link
Copy Markdown
Contributor

Description

ProcessCompressionParams (lib/jpegli/encode.cc) validates each component's component_index and sampling factors but never range-checks comp_info[c].quant_tbl_no, a field of the public jpeg_compress_struct that the application can set to any int. InitQuantizer (lib/jpegli/quant.cc) then reads cinfo->quant_tbl_ptrs[quant_tbl_no], a four-entry array (NUM_QUANT_TBLS), guarded only by a == nullptr check, so an out-of-range index reads a pointer past the array and dereferences the non-null garbage it finds.

Trigger: set cinfo.comp_info[0].quant_tbl_no above NUM_QUANT_TBLS - 1 and call jpegli_start_compress. Under ASan this is an out-of-bounds read / SEGV at InitQuantizer, reached via jpegli_start_compress -> InitCompress -> InitQuantizer.

Noticed comparing the encoder to libjpeg-turbo, which bounds the same value in jcdctmgr.c (qtblno < 0 || qtblno >= NUM_QUANT_TBLS), and to the decoder DQT path, which already bounds quant_table_index at NUM_QUANT_TBLS - 1. The fix adds the missing check in the central per-component validation loop, next to the existing sampling-factor check and matching the sibling DC/AC Huffman index checks in CopyHuffmanTable. Added a regression test that sets an out-of-range quant_tbl_no and expects a clean error instead of the out-of-bounds access; error_handling_test, encode_api_test and transcode_api_test all pass under ASan.

Pull Request Checklist

  • CLA Signed: Have you signed the Contributor License Agreement (individual or corporate, as appropriate)? Only contributions from signed contributors can be accepted.
  • Authors: Have you considered adding your name to the AUTHORS file?
  • Code Style: Have you ensured your code adheres to the project's coding style guidelines? You can use ./ci.sh lint for automatic code formatting.

ProcessCompressionParams validates each component's sampling factors and
component index but never range-checks comp_info[c].quant_tbl_no, an
application-settable field of the public jpeg_compress_struct. InitQuantizer
later reads cinfo->quant_tbl_ptrs[quant_tbl_no], a NUM_QUANT_TBLS element
array, guarded only by a null check, so an out-of-range value reads a pointer
past the array and dereferences it (ASan: out-of-bounds read reached via
jpegli_start_compress -> InitCompress -> InitQuantizer).

libjpeg-turbo bounds-checks the same value in jcdctmgr.c, and the decoder DQT
path already bounds it at NUM_QUANT_TBLS - 1. Add the missing range check in
the central per-component validation loop, matching those and the sibling
DC/AC Huffman index checks.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant