Skip to content

Commit 542bc41

Browse files
authored
Add robust stack validation (#3244)
***NO_CI***
1 parent 577be8f commit 542bc41

File tree

1 file changed

+95
-2
lines changed

1 file changed

+95
-2
lines changed

src/CLR/Core/Interpreter.cpp

Lines changed: 95 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1047,6 +1047,11 @@ HRESULT CLR_RT_Thread::Execute_IL(CLR_RT_StackFrame &stackArg)
10471047
bool fCondition;
10481048
bool fDirty = false;
10491049

1050+
#if defined(NANOCLR_OPCODE_STACKCHANGES)
1051+
// Track stack depth for validation
1052+
CLR_RT_HeapBlock *evalPosBeforeOp = nullptr;
1053+
#endif
1054+
10501055
READCACHE(stack, evalPos, ip, fDirty);
10511056

10521057
while (true)
@@ -1093,8 +1098,62 @@ HRESULT CLR_RT_Thread::Execute_IL(CLR_RT_StackFrame &stackArg)
10931098
#if defined(NANOCLR_OPCODE_STACKCHANGES)
10941099
if (op != CEE_PREFIX1)
10951100
{
1096-
NANOCLR_CHECK_HRESULT(
1097-
CLR_Checks::VerifyStackOK(*stack, &evalPos[1], c_CLR_RT_OpcodeLookup[op].StackChanges()));
1101+
// Capture stack position before opcode execution
1102+
evalPosBeforeOp = evalPos;
1103+
1104+
const CLR_RT_OpcodeLookup &opcodeInfo = c_CLR_RT_OpcodeLookup[op];
1105+
CLR_INT32 stackPop = opcodeInfo.StackPop();
1106+
CLR_INT32 stackPush = opcodeInfo.StackPush();
1107+
CLR_INT32 stackChange = opcodeInfo.StackChanges();
1108+
1109+
// Skip validation for opcodes with variable stack behavior (VarPop/VarPush)
1110+
// These include CEE_CALL, CEE_CALLVIRT, CEE_RET, CEE_NEWOBJ which have stack
1111+
// changes that depend on method signatures and cannot be validated statically
1112+
// Note: VarPop and VarPush are both encoded as 0, so check if EITHER is 0
1113+
bool isVariableStackOp = (stackPop == 0 || stackPush == 0);
1114+
1115+
if (!isVariableStackOp)
1116+
{
1117+
// Validate we have enough items on stack for this opcode to pop
1118+
// evalPos points to the slot AFTER the top item, so &evalPos[0] is top item
1119+
// stack->m_evalStack is the base, so (evalPos - stack->m_evalStack) is current depth
1120+
CLR_INT32 currentDepth = (CLR_INT32)(evalPos - stack->m_evalStack) + 1;
1121+
1122+
if (stackPop > 0 && currentDepth < (CLR_INT32)stackPop)
1123+
{
1124+
// Stack underflow: trying to pop more items than available
1125+
CLR_Debug::Printf(
1126+
"STACK UNDERFLOW PRE-CHECK: Opcode %s (0x%X) needs %d items but only %d available\r\n",
1127+
opcodeInfo.Name(),
1128+
(int)op,
1129+
(int)stackPop,
1130+
(int)currentDepth);
1131+
NANOCLR_SET_AND_LEAVE(CLR_E_STACK_UNDERFLOW);
1132+
}
1133+
1134+
// Validate we have enough space for items this opcode will push
1135+
// After popping stackPop items and pushing stackPush items, new depth will be:
1136+
// currentDepth - stackPop + stackPush = currentDepth + stackChange
1137+
CLR_INT32 projectedDepth = currentDepth + stackChange;
1138+
CLR_INT32 maxDepth = (CLR_INT32)(stack->m_evalStackEnd - stack->m_evalStack);
1139+
1140+
if (projectedDepth > maxDepth)
1141+
{
1142+
// Stack overflow: would exceed maximum stack depth
1143+
CLR_Debug::Printf(
1144+
"STACK OVERFLOW PRE-CHECK: Opcode %s (0x%X) would result in depth %d, max is %d\r\n",
1145+
opcodeInfo.Name(),
1146+
(int)op,
1147+
(int)projectedDepth,
1148+
(int)maxDepth);
1149+
NANOCLR_SET_AND_LEAVE(CLR_E_STACK_OVERFLOW);
1150+
}
1151+
}
1152+
else
1153+
{
1154+
// For variable stack opcodes, set evalPosBeforeOp to nullptr to skip post-validation
1155+
evalPosBeforeOp = nullptr;
1156+
}
10981157
}
10991158
#endif
11001159

@@ -2463,6 +2522,10 @@ HRESULT CLR_RT_Thread::Execute_IL(CLR_RT_StackFrame &stackArg)
24632522
#ifndef NANOCLR_NO_IL_INLINE
24642523
if (stack->m_inlineFrame)
24652524
{
2525+
// Inline return - skip stack validation as the context is restored by PopInline()
2526+
#if defined(NANOCLR_OPCODE_STACKCHANGES)
2527+
evalPosBeforeOp = nullptr; // Disable post-validation for inline return
2528+
#endif
24662529
stack->m_evalStackPos = evalPos;
24672530

24682531
stack->PopInline();
@@ -4241,6 +4304,36 @@ HRESULT CLR_RT_Thread::Execute_IL(CLR_RT_StackFrame &stackArg)
42414304
#undef OPDEF
42424305
}
42434306

4307+
#if defined(NANOCLR_OPCODE_STACKCHANGES)
4308+
// Post-execution validation: verify stack depth changed by expected amount
4309+
if (op != CEE_PREFIX1 && evalPosBeforeOp != nullptr)
4310+
{
4311+
const CLR_RT_OpcodeLookup &opcodeInfo = c_CLR_RT_OpcodeLookup[op];
4312+
CLR_INT32 expectedChange = opcodeInfo.StackChanges();
4313+
CLR_INT32 actualChange = (CLR_INT32)(evalPos - evalPosBeforeOp);
4314+
4315+
if (actualChange != expectedChange)
4316+
{
4317+
// Stack depth mismatch: opcode didn't change stack as expected
4318+
CLR_Debug::Printf(
4319+
"STACK DEPTH MISMATCH POST-CHECK: Opcode %s (0x%X) expected change %d but actual change was %d\r\n",
4320+
opcodeInfo.Name(),
4321+
(int)op,
4322+
(int)expectedChange,
4323+
(int)actualChange);
4324+
CLR_Debug::Printf(
4325+
" Stack before: %d items, after: %d items\r\n",
4326+
(int)(evalPosBeforeOp - stack->m_evalStack) + 1,
4327+
(int)(evalPos - stack->m_evalStack) + 1);
4328+
4329+
// This is a critical error indicating a bug in the opcode implementation
4330+
NANOCLR_SET_AND_LEAVE(CLR_E_STACK_UNDERFLOW);
4331+
}
4332+
4333+
evalPosBeforeOp = nullptr;
4334+
}
4335+
#endif
4336+
42444337
#if defined(NANOCLR_ENABLE_SOURCELEVELDEBUGGING)
42454338
if (stack->m_flags & CLR_RT_StackFrame::c_HasBreakpoint)
42464339
{

0 commit comments

Comments
 (0)