Bound the NSMethodSignature type-rewrite buffer to prevent stack overflow#598
Conversation
…flow
* Source/NSMethodSignature.m ([NSMethodSignature _initWithObjCTypes:]):
Cap the on-stack working buffer at 4096 bytes and fall back to malloc
for larger buffers so that a pathologically long type encoding cannot
force an unbounded alloca and overflow the stack.
* Tests/base/NSMethodSignature/alloca_cap.m: New regression test
covering the alloca path, the cap boundary, the first heap-path case,
and a signature whose working buffer would exceed any reasonable
stack limit.
Validated on Ubuntu 24.04 + clang 18 + libobjc2:
fixed: 4/4 assertions pass.
unfixed: 3/4 assertions pass; the 1.5M-arg case aborts with
SIGSEGV from alloca overflow, exactly as the test is
designed to detect.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
rfm
left a comment
There was a problem hiding this comment.
I haven't looked at the testcases, but we should probably use the GS_BEGINITEMBUF and GS_ENDITEMBUF macros for this,
Each case now also verifies -numberOfArguments so the test proves the parser walked the whole type string, not just that some non-nil signature object was returned. The counts are deterministic (2 for "v@:", 2+N for the int-arg cases) so the assertions remain portable. Re-validated on Ubuntu 24.04 + clang 18: 4/4 pass with fix.
|
Actually, thinking about it ... it seems the solution here is completely wrong. |
rfm
left a comment
There was a problem hiding this comment.
Maybe returning nil is enough, but an exception seems clearer. I'm not actually sure.,
Per rfm's review of PR gnustep#598, replace the hand-rolled alloca-cap / malloc-fallback in -[NSMethodSignature _initWithObjCTypes:] with the existing GS_BEGINITEMBUF / GS_ENDITEMBUF pair from Source/GSPrivate.h, which is already the libs-base idiom for "small on the stack, big on the heap" temporary buffers. Net effect on behaviour is the same (the heap threshold is now GS_MAX_OBJECTS_FROM_STACK instead of 4096, and this code path is only reached once per unique type string thanks to the cache in +signatureWithObjCTypes:, so the extra heap usage is negligible). * Source/NSMethodSignature.m: use GS_BEGINITEMBUF; drop the locally-held heap pointer and the in-function cap constant. * ChangeLog: update entry. Re-validated on Ubuntu 24.04 + clang 18: 4/4 pass with fix, 3/4 pass without (SIGSEGV on 1.5M-arg case as designed).
|
I think an exception is more informative.
TW.
…On Wed, Apr 15, 2026 at 10:09 AM rfm ***@***.***> wrote:
***@***.**** commented on this pull request.
Maybe returning nil is enough, but an exception seems clearer. I'm not
actually sure.,
—
Reply to this email directly, view it on GitHub
<#598 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BSTQQ7EMITZLKZ2I2BPCEDD4V6JX7AVCNFSM6AAAAACX2H3LXSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHM2DCMJUGI3DENRUGE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Good call — done in 681cb5b. The fix is now just blen = (strlen(t) + 1) * 16;
GS_BEGINITEMBUF(ret, blen, char)
end = ret + blen;
... existing body ...
((char*)_methodTypes)[alen + rlen] = '\0';
GS_ENDITEMBUF()which is a significantly smaller patch than the one you saw. Re-validated on Ubuntu 24.04 + clang 18: 4/4 pass with the fix, 3/4 pass without (the 1.5M-arg case still SIGSEGVs on an unpatched build, exactly as the test is designed to detect). The revised alloca_cap test was pushed in 4026964 just before this — it now also asserts |
Per rfm's follow-up review of PR gnustep#598, no compiler-emitted method type encoding approaches the working-buffer size the old alloca was sized for, so an encoding whose (strlen+1)*16 exceeds a small cap is almost certainly an attack. Reject it outright rather than allocating anything for it. This replaces the previous GS_BEGINITEMBUF-based heap fallback with a tiny exception-based guard. * Source/NSMethodSignature.m ([NSMethodSignature _initWithObjCTypes:]): Raise NSInvalidArgumentException when the working buffer would exceed 4096 bytes; otherwise keep the original alloca path unchanged. Remove the GSPrivate.h import and the GS_BEGINITEMBUF wrapper from the previous commit. ([NSMethodSignature signatureWithObjCTypes:]): Release the cache lock on exceptions thrown from _initWithObjCTypes:. This bug was dormant before because the init path never raised; the new cap check makes it observable, so fix it in the same change to keep the cap check safe. * Tests/base/NSMethodSignature/alloca_cap.m: Case 3 (253-arg, just past the cap) and case 4 (1.5M-arg) now use PASS_EXCEPTION on NSInvalidArgumentException instead of expecting a non-nil signature. Cases 1 and 2 are unchanged. Re-validated on Ubuntu 24.04 + clang 18 + libobjc2: fixed: 4/4 pass. unfixed: 2/4 pass, case 3 fails (no exception raised), case 4 aborts with SIGSEGV from the unbounded alloca, exactly as the test is designed to detect.
|
Agreed on both counts — raising is the right answer, and you're right that no real compiler produces encodings near that length. Pushed a21982d. The new patch is much smaller than either previous version. The entire fix is now: blen = (strlen(t) + 1) * 16;
/* No compiler-emitted type encoding approaches this size, so
* reject an excessively long one outright rather than allocating
* an arbitrary amount of stack for it.
*/
if (blen > 4096)
{
[NSException raise: NSInvalidArgumentException
format: @"Method signature type encoding is too long"];
}
ret = alloca(blen);4096 bytes covers type strings up to ~255 characters, which is orders of magnitude more than any real method signature needs. One incidental fix bundled inValidating the first version surfaced a pre-existing latent bug: Regression testCases 3 and 4 (253-arg and 1.5M-arg) now use
So each new assertion still discriminates fixed and unfixed. |
What
-[NSMethodSignature _initWithObjCTypes:]rewrites the caller-supplied type encoding into a temporary buffer sized(strlen(t) + 1) * 16. That buffer was always taken from the stack viaalloca, so a sufficiently long type encoding could force an arbitrarily large stack allocation and push past the guard page.This PR caps the on-stack allocation at 4096 bytes and falls back to
mallocfor buffers above the cap. 4096 bytes covers type encodings up to ~255 characters, which is well beyond any real Objective-C method signature. The fast path is unchanged for ordinary signatures; only pathologically long encodings touch the heap path, and the allocated buffer is freed on the single exit from the block.Why
A long type encoding is attacker-influenceable anywhere method signatures are built from untrusted input (e.g. unarchiving, IPC with a compromised peer). Unbounded
allocaon such input is a denial-of-service vector at minimum and — depending on stack layout — potentially more. The cap eliminates the DoS without changing any observable behaviour for legitimate callers.Regression test
Tests/base/NSMethodSignature/alloca_cap.m— four assertions:"v@:"signature still parses (alloca fast path).Validated on Ubuntu 24.04 + clang 18 + libobjc2:
The test uses only plain
main()+ inlinePASS()with a static C helper for input construction, following the shape ofTests/base/NSJSONSerialization/depth.m.Scope
One finding, one PR. No unrelated changes.