Hi, I reproduced the overflow described in this thread on upstream head e5bb73f8, with AddressSanitizer and UndefinedBehaviorSanitizer enabled. In tape_buffered_peek (src/util.c), a read of io_block_size bytes at the end of input_buffer writes past the buffer. ASan reports a heap-buffer-overflow WRITE of 512 bytes that starts at the end of the 1024-byte input buffer. It occurred in 3 of 3 runs where standard input is a pipe and the data arrives in pieces: 256 bytes, a pause, 512 bytes, a pause, 512 bytes, a pause, then the archive. The same bytes sent in one burst did not trigger it.
I am sending a separate, alternative fix for src/util.c, attached, for comparison with the patch above. When a read would extend past input_buffer_size, it moves the unconsumed bytes to the start of the buffer before reading. Reads are unchanged when the buffer has room. The attached patch does not include a test. With the attached patch applied, the same timed-pipe input ran with no sanitizer report and exit status 0, and the extracted file was present. The full test suite under the same sanitizer build runs 18 tests, with 1 failure: linktime.at:17. It is a LeakSanitizer report from copypass.c (process_copy_pass), and the same test also fails on unpatched head in our earlier run. I have not run the patch above against these inputs. The maintainers may prefer either fix. Vivek Kumar
0001-tape_buffered_peek-check-the-end-of-each-read-agains.patch
Description: Binary data
