Fix heap buffer overflow in libopenapv encoder#212
Conversation
Add bounds checking to BSW_FLUSH_4BYTE and BSW_FLUSH_8BYTE macros in oapv_vlc.c to prevent writing past the end of the bitstream buffer during VLC encoding. Add bounds checking in enc_frame in oapv.c to ensure the cumulative tile bitstream size does not exceed the target bitstream buffer end before copying tile bitstreams. These changes prevent heap buffer overflows in the encoder. Bug: 501452526 Test: Manual verification with PoC binaries on Cuttlefish Flag: EXEMPT CVE_FIX Change-Id: Ib40bc500096b6fda93e5802d97b306e4320ba6eb
|
@kpchoi Please review and merge if appropriate |
fkyslov
force-pushed
the
fix-heap-overflow-501452526
branch
from
4999e46 to
577ee9c
Compare
| /****************************************************/ | ||
|
|
||
| for(int i = 0; i < ctx->num_tiles; i++) { | ||
| oapv_assert_gv(bs_tile_pos + ctx->tile[i].bs_size <= bs->end, ret, OAPV_ERR_OUT_OF_BS_BUF, ERR); |
There was a problem hiding this comment.
this function is for encoding process, so that the tile[i].bs_size has correct number.
If the address is located in outside of available bitstream buffer, enc_tile() returns error.
There was a problem hiding this comment.
the check at enc_tile() happens after enc_tile_comp where the buffer overrun can happen.
There was a problem hiding this comment.
Also, each individual tile buffer (tile[i].bs_size) may be fit, but when assembling all encoded tiles into common "bs" we need to check if the sum of tiles doesn't exceed the common "bs" size. I.e. those are different checks.
| @@ -37,23 +37,31 @@ | |||
| #if ENABLE_ENCODER | |||
| /////////////////////////////////////////////////////////////////////////////// | |||
| #define BSW_FLUSH_4BYTE(bs) { \ | |||
There was a problem hiding this comment.
This function is called anywhere. It will be removed.
| (bs)->leftbits = 32; \ | ||
| } | ||
|
|
||
| #define BSW_FLUSH_8BYTE(bs) { \ |
There was a problem hiding this comment.
This function is called anywhere. It will be removed
There was a problem hiding this comment.
It is called in BSW_WRITE_64BITS and BSW_WRITE_64BITS is called in oapve_vlc_ac_coef and oapve_vlc_ac_coef is called in enc_tile_comp, or am I missing something?
There was a problem hiding this comment.
Again, one branch in vlc loop code, it makes noticeable speed-down.
It is assumed that the input buffer should be allocated large enough.
This code should be reconsidered.
There was a problem hiding this comment.
I'd like to suggest to merge this PR without BSW_WRITE_XX changes.
I think we can reconsider BSW_WRITE_XX code changes if some noticeable issue occurs.
2 participants
Add bounds checking to BSW_FLUSH_4BYTE and BSW_FLUSH_8BYTE macros in oapv_vlc.c to prevent writing past the end of the bitstream buffer during VLC encoding.
Add bounds checking in enc_frame in oapv.c to ensure the cumulative tile bitstream size does not exceed the target bitstream buffer end before copying tile bitstreams.
Change-Id: Ib40bc500096b6fda93e5802d97b306e4320ba6eb