Bound exp-Golomb prefix and KPARAM macros in VLC coefficient readers

[fkyslov]

Summary

Bounds exp-Golomb prefix length and clamps KPARAM_* macros in the VLC coefficient readers:

• dec_vlc_read_kparam0() & dec_vlc_read_1bit_read() (src/oapv_vlc.c):
  • Bound prefix length k < 30 inside and after the prefix loops and accumulate symbol as u32 so malformed bitstreams with long zero runs are rejected before k or symbol can overflow.
• KPARAM_DC, KPARAM_AC, KPARAM_RUN (src/oapv_vlc.h):
  • Clamp KPARAM_* macros using oapv_clip3() to [OAPV_KPARAM_*_MIN, OAPV_KPARAM_*_MAX].

Testing

• Verified all 24 ctest unit/conformance tests pass with AddressSanitizer and UndefinedBehaviorSanitizer.

[kpchoi]

Firstly, Regarding 679 and 719 of your patch,

The in-loop k checks added to dec_vlc_read_kparam0() and dec_vlc_read_1bit_read() are not needed. These loops cannot run forever, even on a malformed bitstream.

• At the end of the buffer, BSR_FLUSH_1BYTE() does not read past it. It fills code with all 1-bits (~(u64)0) and sets is_eob.
• The next BSR_READ_1BIT() then returns 1, so flag becomes true and the loop exits through break.
• Inside these loops k is only incremented and never used as a shift count. The existing check after the loop rejects too large a k before it is used.

The error is detected later instead (deferred error checking). The callers test BSR_IS_UNEXPECTED_EOB() after parsing the coefficients and return OAPV_ERR_MALFORMED_BITSTREAM. This is intentional. These two functions are the k = 0 paths and are the ones called most often in coefficient decoding, so a per-bit check in their loops would slow down decoding.

Comments explaining this will be added near these loops.

[kpchoi]

The oapv_clip3() change to the KPARAM_* macros is not needed either. Their arguments are never negative, and OAPV_KPARAM_*_MIN is 0, so the lower clamp can never take effect:

• KPARAM_DC() gets abs_dc_diff, which is at least 1 at that point.
• KPARAM_AC() gets level, which is at least 1.
• KPARAM_RUN() gets run, which has already been range-checked to [0, OAPV_BLK_D - scan_pos_offset].
• On the encoder side, only absolute values are passed.

These macros are evaluated for every coefficient. In VLC decoding even one extra branch or compare per coefficient noticeably affects decoding speed, so oapv_clip3() is intentionally not used here.

[kpchoi]

Changing symbol from int to u32 is not needed for correctness. The additions are already done in unsigned arithmetic (int + u32, int + u64), so there is no signed overflow. With the k check after the loop, the result always fits in int.

Still, the change is fine to keep as a cosmetic change. It matches dec_vlc_read(), which already uses u32 symbol, and it keeps code reviewers and coding agents from mistaking this pattern for a signed overflow.

[kpchoi]

This is a correct comparison. thanks.

[kpchoi]

This line is strongly suggested to be removed because of the reason commented.

[kpchoi]

This line is strongly suggested to be removed because of the reason commented.

[kpchoi]

This 3 line are not required to be changed because of the reason commented.