Repository navigation
Conversation
Since catalogs are written with 64 KiB pages (4d7587e), block_off in sas7bcat_begin_data wraps negative once the blocks before it pass 32 KiB. The page-size check then compares a sign-extended value, so the next block is either dropped or copied to &page[block_off], before the start of the page buffer. sas7bcat_block_for_label_set has the same problem within a block: a label of 32768 bytes or more gives memcpy a negative length, and a string value of 32752 bytes or more moves lbp1 backwards, so the following value entries are written before the block. Do this arithmetic in size_t and narrow only when storing the 2-byte fields, which the reader decodes as uint16_t.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
sas7bcat_begin_datakeeps the write offset into the catalog's block page in anint16_t. Since #417 catalogs also get 64 KiB pages, so once the blocks laid out so far pass 32 KiB the offset goes negative and the page-size check sees a sign-extended value: a small next block is silently dropped, and one at least as large as the wrapped distance is copied to&page[block_off], up to 32 KiB before the page buffer (two formats of about 800 short labels each are enough, and under ASan it faults atreadstat_sas7bcat_write.c:245).sas7bcat_block_for_label_setnarrows the same way inside a block: a label of 32768 bytes makes the labelmemcpylength negative, and a string value of 32752 bytes or more stepslbp1backwards so the following value entries land before the block.The file fields are 2 bytes and the reader already decodes them as
uint16_t(sas_read2), so the writer now does its layout arithmetic insize_tand narrows only when storing those fields. The new round-trip test intest_sas.ccovers all three cases for 32- and 64-bit catalogs; on the current code it loses the third of three 20 KiB formats, crashes on the long label and writes an unreadable catalog for the long string value.