Ensure correct integer promotion when adding #268
Conversation
Code Quality (Avoid Integer Overflow): Ensure correct order of integer size promotion when doing additions by placing the mz_uint64 value first or explicitly casting the first value to mz_uint64.
Update miniz_zip.c
Additional parameter checks
| @@ -203,6 +203,10 @@ tinfl_status tinfl_decompress(tinfl_decompressor *r, const mz_uint8 *pIn_buf_nex | |||
| size_t out_buf_size_mask = (decomp_flags & TINFL_FLAG_USING_NON_WRAPPING_OUTPUT_BUF) ? (size_t)-1 : ((pOut_buf_next - pOut_buf_start) + *pOut_buf_size) - 1, dist_from_out_buf_start; | |||
|
|
|||
| /* Ensure the output buffer's size is a power of 2, unless the output buffer is large enough to hold the entire output file (in which case it doesn't matter). */ | |||
| if ((!pOut_buf_start) || (!pOut_buf_next) || (!pIn_buf_size) || (!pOut_buf_size)) | |||
There was a problem hiding this comment.
This might impact performance but YMMV
There was a problem hiding this comment.
This part of the patch is wrong: pOut_buf_next can legitimately be NULL here, and this change results in decompression failure with valid files.
There was a problem hiding this comment.
I'm hitting pOut_buf_start == NULL with tinfl_decompress called from tinfl_decompress_mem_to_heap
There was a problem hiding this comment.
I see that in tinfl_decompress_mem_to_heap, the first iteration of the main for loop has pBuf set to NULL and the resulting call to tinfl_decompress will have both pOut_buf_start and pOut_buf_next set to NULL. tinfl_decompress responds to this with TINFL_STATUS_HAS_MORE_OUTPUT if the check is eliminated, which is correct behaviour. Calling this function with either buffer being NULL while the other is valid also results in a valid error code. Calling it with either buffer length pointer being NULL results in a read access violation before the test. I therefore think it would be best to:
- Remove the test for
NULLforpOut_buf_startandpOut_buf_next, and, - Rework the initialization to test for
NULLforpIn_buf_sizeandpOut_buf_size.
Remove parameter check that breaks tinfl_decompress_mem_to_heap() (see discussion on pull request richgel999#268)
When adding multiple mz_uint32 values (or a mix of mz_uint32 and mz_uint64 values) to a mz_uint64 value, the mz_uint32 values should be promoted to mz_uint64 before the addition is done. To ensure this is the case, the mz_uint64 value should be placed first, or, if all values are mz_uint32, then the first one should be cast to mz_uint64.