Fix IJW OverflowException with 17+ by-ref parameters#127182
Fix IJW OverflowException with 17+ by-ref parameters#127182jkoritzinsky wants to merge 5 commits intodotnet:mainfrom
Conversation
StubSigBuilder::EnsureEnoughQuickBytes only doubled the buffer size once, so when the required size exceeded 2x the current buffer, the CQuickBytes inline buffer (512 bytes) overflowed into the adjacent m_nItems field. This was triggered by PR dotnet#106000 which changed ConvertToInternalSignature to preserve custom modifiers (bSkipCustomModifier=FALSE). Each preserved modifier adds 10 bytes (ELEMENT_TYPE_CMOD_INTERNAL + required byte + 8-byte TypeHandle pointer) to the signature. For 18+ parameters with custom modifiers, the total signature exceeded 512 bytes, causing a buffer overflow that corrupted m_nItems and led to an OverflowException. The fix loops the doubling until the buffer is large enough. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes an IJW (C++/CLI) reverse P/Invoke signature-building buffer growth bug that can corrupt signature state and surface as an OverflowException when native code calls managed methods with many by-ref parameters (and preserved custom modifiers).
Changes:
- Update
StubSigBuilder::EnsureEnoughQuickBytesto keep doubling until the requested append size fits. - Add an IJW regression test that exercises a native→managed call with 18 by-ref parameters and validates the returned sum.
- Extend the IJW native/managed mixed-mode test DLL with helper entrypoints to drive the scenario.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/coreclr/vm/stubgen.cpp | Fixes the QuickBytes growth logic to prevent signature buffer overwrite/corruption. |
| src/tests/Interop/IJW/NativeCallingManaged/NativeCallingManaged.cs | Adds a managed-side regression test case invoking the new IJW scenario via reflection. |
| src/tests/Interop/IJW/IjwNativeCallingManagedDll/IjwNativeCallingManagedDll.cpp | Adds a mixed-mode native→managed call chain with 18 by-ref parameters to reproduce the prior failure. |
- Use S_SIZE_T for checked arithmetic in EnsureEnoughQuickBytes to prevent infinite loop or undersized allocation on SIZE_T overflow. - Remove unnecessary NativeSum18ByRefEntryPoint DLL export. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
| SIZE_T cbNew = max(m_cbSig + cbToAppend, 2 * cbBuffer); | ||
| // Detect integer overflow | ||
| if ((cbNew - m_cbSig) < cbToAppend) | ||
| COMPlusThrowHR(COR_E_OVERFLOW); |
There was a problem hiding this comment.
The overflow check here can be defeated because the potentially overflowing operations happen before the check (m_cbSig + cbToAppend and 2 * cbBuffer can wrap in SIZE_T). Consider doing the size calculations with S_SIZE_T/ClrSafeInt (including the doubling) and checking IsOverflow() before resizing, instead of relying on post-hoc subtraction with unsigned values.
| SIZE_T cbNew = max(m_cbSig + cbToAppend, 2 * cbBuffer); | |
| // Detect integer overflow | |
| if ((cbNew - m_cbSig) < cbToAppend) | |
| COMPlusThrowHR(COR_E_OVERFLOW); | |
| S_SIZE_T cbRequired = S_SIZE_T(m_cbSig) + S_SIZE_T(cbToAppend); | |
| if (cbRequired.IsOverflow()) | |
| COMPlusThrowHR(COR_E_OVERFLOW); | |
| S_SIZE_T cbDoubled = S_SIZE_T(cbBuffer) * S_SIZE_T(2); | |
| SIZE_T cbNew = cbRequired.Value(); | |
| if (!cbDoubled.IsOverflow()) | |
| { | |
| cbNew = max(cbNew, cbDoubled.Value()); | |
| } |
Note
This PR was authored with the assistance of GitHub Copilot.
Summary
Fixes #127166 — IJW (C++/CLI) reverse P/Invoke throws
OverflowExceptionwhen calling a managed function with 17+ by-ref parameters from native code.Root Cause
StubSigBuilder::EnsureEnoughQuickBytesonly doubled the buffer size once. WhenFunctionSigBuilder::SetSigneeded to store a 524-byte parameter signature (18 by-ref params with custom modifiers), the buffer grew from 256 → 512 bytes — still in theCQuickBytesinline buffer. Writing 524 bytes overflowed the 512-byte inline buffer by 12 bytes into the adjacentm_nItemsfield, corrupting the arg count from 18 to garbage (~0x2800220F). This causedGetSigSize()to compute an overflowedS_UINT32and throwCOR_E_OVERFLOW.Why This Regressed
PR #106000 changed
ConvertToInternalSignatureto preserve custom modifiers (bSkipCustomModifier=FALSE). Each preserved modifier adds 10 bytes (ELEMENT_TYPE_CMOD_INTERNAL+ required byte + 8-byte TypeHandle pointer). For 18+ parameters withNeedsCopyConstructorModifier, the signature exceeded 512 bytes for the first time, triggering the latent buffer overflow inEnsureEnoughQuickBytes.Fix
Loop the doubling in
EnsureEnoughQuickBytesuntil the buffer is large enough to hold the requested data.Regression Test
Added an IJW test with a managed function that has 18 by-ref parameters (
const int&andconst IntWrapper&), called from native#pragma unmanagedcode. The test verifies the correct sum (153) is returned.