Ensure null termination of extended_attrs, width and height#299
Ensure null termination of extended_attrs, width and height#299
Conversation
These are later output via printf in linkysize(), therefore these two strings must always be null terminated. In order to ensure that we can still use `S(width) > 0` to check for their presence, we pretend that the null byte doesn't exist.
nmeum
force-pushed
the
null-termination
branch
from
2ba7ac6 to
a3b7649
Compare
nmeum
force-pushed
the
null-termination
branch
from
a3b7649 to
9c3b1de
Compare
|
Rebased against current main and fixed some additional memory errors related to footnotes. |
|
This fixes issues with footnote numbering that I see since 3.0.1 on OpenBSD with default malloc options (see https://man.openbsd.org/malloc.3 for info; in particular the default enables one level of 'J' where the first part of malloc allocations have 0xdf written over them) that go away if I disable that by setting MALLOC_OPTIONS=j (lowercase). |
|
Aha! Thank you, you unlocked the mental block that was stopping me from resolving this issue.
I build everything with amalloc to catch buffer overflows, but amalloc uses calloc() by default and thus anything that was not terminated was being caught by the amalloced buffer.
I modified amalloc to initialize malloc()ed memory with a non-zero character and a whole bunch of test cases started throwing up red flags.
Again, thank you for this comment; I'm gonna release 3.0.1.2 with this round of changes today.
… On Sep 8, 2025, at 5:10 AM, Stuart Henderson ***@***.***> wrote:
sthen
left a comment
(Orc/discount#299)
<#299 (comment)>
This fixes issues with footnote numbering that I see since 3.0.1 on OpenBSD with default malloc options (see https://man.openbsd.org/malloc.3 <https://man.openbsd.org/malloc.3> for info; in particular the default enables one level of 'J' where the first part of malloc allocations have 0xdf written over them) that go away if I disable that by setting MALLOC_OPTIONS=j (lowercase).
—
Reply to this email directly, view it on GitHub <#299 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAE5AQKCA3CL2GK4OMVAV33RVW3DAVCNFSM6AAAAACF3DP5D2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTENRVHE4TONRYGQ>.
You are receiving this because you are subscribed to this thread.
|
|
you're welcome. more proof that a diverse ecosystem helps get problems fixed :) |
|
btw, just looking over release notes and noticed you've run into a problem that I used to have but eventually found a workaround - if you'd like to export a patch from a github pull request, just add .patch to the main url, e.g.: |
3 participants
This PR contains a follow-up for eda57f5, improving null-termination for extended attributes. Further, it ensures zero initialization of the reference counter used for footnotes. Lastly, it fixes out-of-bounds reads caused by lack of string null termination in the changes added in 6ff6878 (which made width/height a string but didn't ensure their null termination).
Since the generator uses the
S(<field>) > 0idiom to check for their presence, and I didn't want to modify these checks, I used a trick I discovered elsewhere in the codebase to exclude them from the size count. Let me know if you prefer a different solution.Fixes #298