Skip to content

sas7bcat: keep writer block offsets and lengths out of int16_t - #422

Open
ziyziy89 wants to merge 1 commit into
WizardMac:devfrom
ziyziy89:sas7bcat-writer-int16-offsets
Open

ziyziy89 wants to merge 1 commit into
WizardMac:devfrom
ziyziy89:sas7bcat-writer-int16-offsets

Conversation

@ziyziy89

Copy link
Copy Markdown
Contributor

sas7bcat_begin_data keeps the write offset into the catalog's block page in an int16_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 at readstat_sas7bcat_write.c:245). sas7bcat_block_for_label_set narrows the same way inside a block: a label of 32768 bytes makes the label memcpy length negative, and a string value of 32752 bytes or more steps lbp1 backwards 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 in size_t and narrows only when storing those fields. The new round-trip test in test_sas.c covers 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.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant