Skip to content

Bound the NSMethodSignature type-rewrite buffer to prevent stack overflow#598

Merged
rfm merged 4 commits intognustep:masterfrom
DTW-Thalion:fix/method-sig-alloca-cap
Apr 15, 2026
Merged

Bound the NSMethodSignature type-rewrite buffer to prevent stack overflow#598
rfm merged 4 commits intognustep:masterfrom
DTW-Thalion:fix/method-sig-alloca-cap

Conversation

@DTW-Thalion
Copy link
Copy Markdown
Contributor

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 via alloca, 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 malloc for 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 alloca on 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:

  1. A trivial "v@:" signature still parses (alloca fast path).
  2. A 252-int-arg signature whose working buffer is exactly 4096 bytes still parses (last alloca case).
  3. A 253-int-arg signature whose working buffer is just over the cap still parses (first heap case).
  4. A 1.5M-int-arg signature whose working buffer would require ~24 MB of stack still parses (deep heap path). Without the cap this case performs an alloca well past any reasonable stack limit, which is how the test distinguishes fixed from unfixed builds.

Validated on Ubuntu 24.04 + clang 18 + libobjc2:

build result
with fix 4/4 PASS
without fix 3/4 PASS, then SIGSEGV on case 4 from alloca overflow

The test uses only plain main() + inline PASS() with a static C helper for input construction, following the shape of Tests/base/NSJSONSerialization/depth.m.

Scope

One finding, one PR. No unrelated changes.

…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>
@DTW-Thalion DTW-Thalion requested a review from rfm as a code owner April 15, 2026 13:32
Copy link
Copy Markdown
Contributor

@rfm rfm left a comment

Choose a reason for hiding this comment

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

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.
@rfm
Copy link
Copy Markdown
Contributor

rfm commented Apr 15, 2026

Actually, thinking about it ... it seems the solution here is completely wrong.
The reason we aren't using the macros already is because an overflow seems practically impossible (how could we get such a long signature)?
Surely, if we get an excessively long signature the thing to do is raise an exception rather than trying to handle it (and potentially, assuming the long signature is due to a deliberate attack, running out of heap space I suppose).

Copy link
Copy Markdown
Contributor

@rfm rfm left a comment

Choose a reason for hiding this comment

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

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).
@DTW-Thalion
Copy link
Copy Markdown
Contributor Author

DTW-Thalion commented Apr 15, 2026 via email

@DTW-Thalion
Copy link
Copy Markdown
Contributor Author

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. GSPrivate.h is newly imported for the macro. The stack/heap threshold is now GS_MAX_OBJECTS_FROM_STACK rather than the ad-hoc 4096-byte cap I'd written; since +signatureWithObjCTypes: caches by pointer, each unique type string only reaches this path once per process, so the extra heap usage is immaterial.

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 [sig numberOfArguments] is exactly 2 + N in each case so the test proves the parser walked the whole type string rather than just that some non-nil signature came back.

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.
@DTW-Thalion
Copy link
Copy Markdown
Contributor Author

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. GSPrivate.h import and the GS_BEGINITEMBUF wrapper from the previous commit are both gone.

One incidental fix bundled in

Validating the first version surfaced a pre-existing latent bug: +signatureWithObjCTypes: holds cacheTableLock across the [[self alloc] _initWithObjCTypes: t] call, and the lock was never released on the exception path. That was dormant before because nothing inside _initWithObjCTypes: ever raised — the new cap check makes it observable as a deadlock on the next call to +signatureWithObjCTypes: after a rejected encoding. I added a one-shot NS_DURING/NS_HANDLER around the locked region that releases the lock and re-raises. Happy to split this out into a separate PR if you'd prefer, but it's genuinely prerequisite to the cap check working, so it felt cleaner to land both in one commit.

Regression test

Cases 3 and 4 (253-arg and 1.5M-arg) now use PASS_EXCEPTION on NSInvalidArgumentException; cases 1 and 2 still expect a successful parse with matching numberOfArguments. Re-validated on Ubuntu 24.04 + clang 18 + libobjc2:

build result
with fix 4/4 pass
without fix 2/4 pass, case 3 fails (no exception raised on the 253-arg input), case 4 aborts with SIGSEGV from the unbounded alloca

So each new assertion still discriminates fixed and unfixed.

@rfm rfm merged commit 98524e4 into gnustep:master Apr 15, 2026
8 checks passed
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