Skip to content

fix encoding/decoding data size bug (crasher)#434

Merged
fredkiefer merged 1 commit into
masterfrom
para-style-coding1
May 25, 2026
Merged

fix encoding/decoding data size bug (crasher)#434
fredkiefer merged 1 commit into
masterfrom
para-style-coding1

Conversation

@rfm
Copy link
Copy Markdown
Contributor

@rfm rfm commented May 25, 2026

The crashing issue was that we were deserialising an array of NSInteger (host pointer length values) into an array of NSTextTabType (integer size values). On any machine where an integer and a pointer are different lengths, this causes buffer overflow corrupting the stack.

Fundamentally, when serialising/deserialising using buffers of NSTextTabType the methods should be told what type to use by using @encode(NSTextTabType) or perhaps @encode(typeof(variable)) though the latter is compiler dependent.

@rfm rfm requested a review from fredkiefer as a code owner May 25, 2026 18:47
@fredkiefer
Copy link
Copy Markdown
Member

The change itself looks good to me, where I am unsure is, whether we still would need the if statement for the old version of this class. When there are archives out there with the initial int encoding will they be decoded correctly? Also we now may have NSInteger archives out there will they be also be decoded with the new code?
This may be the wrong questions, maybe the cases where the old code worked are the ones where all these values have the same size and in this case the new code will handle them as well. I just wanted to have this cleared before merging this code.

@rfm
Copy link
Copy Markdown
Contributor Author

rfm commented May 25, 2026

Archiving/unarchiving copes well with different word sizes and byte orders, so I think the if statement was never needed. The archive format records the size of a stored integer (and stores in big endian), so when we try to decode an integer with a different size to the size in the archive, the data is adjusted to match the requirement as it is decoded. The only time we should get an exception is if the adjustment is impossible (eg we try to decode the value 65536 into a two byte integer). Since the data we are storing contains values from an enumeration, no archive should contain large values even if it has each value stored in 8 bytes.

Copy link
Copy Markdown
Member

@fredkiefer fredkiefer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the explanation!

@fredkiefer fredkiefer merged commit 35100f4 into master May 25, 2026
4 checks passed
@fredkiefer fredkiefer deleted the para-style-coding1 branch May 25, 2026 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants