Skip to content

aCalc: fix off-by-two in INC() runtime stack-overflow guard#40

Open
physwkim wants to merge 1 commit into
epics-modules:masterfrom
physwkim:fix/acalc-inc-stack-overflow
Open

aCalc: fix off-by-two in INC() runtime stack-overflow guard#40
physwkim wants to merge 1 commit into
epics-modules:masterfrom
physwkim:fix/acalc-inc-stack-overflow

Conversation

@physwkim

Copy link
Copy Markdown

Problem

The aCalc value stack is calloc(ACALC_STACKSIZE) with top = &stack[1],
so the last writable element is stack[ACALC_STACKSIZE-1]. INC()
advances ps, then guards with

if ((ps-top)>ACALC_STACKSIZE) { ... overflow ... }
else { (ps)->numEl = -1; (ps)->sourceDouble = -1; }

Because ps addresses stack[1 + (ps-top)], the write lands out of
bounds once (ps-top) reaches ACALC_STACKSIZE-1. The guard admits
indices ACALC_STACKSIZE and ACALC_STACKSIZE+1 before it trips —
an off-by-two heap overflow of two stackElements.

The postfix compiler caps a purely scalar expression at
ACALC_STACKSIZE, so scalar depth alone never reaches the runtime guard.
But NDERIV/DERIV/FITPOLY push two or three scratch elements at run
time (e.g. NDERIV does DEC then three INCs in aCalcPerform.c) that
the compiler's per-opcode runtime_effect accounting models only as a
net delta, not the transient peak. A deep-but-compiler-accepted
expression such as

1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(NDERIV(AA,2)))))))))))))))))

therefore passes aCalcPostfix() yet drives the runtime stack past its
end, writing out of bounds and crashing the IOC (SIGSEGV).

Fix

Guard on (ps-top) > ACALC_STACKSIZE-2, so INC() rejects before the
write can exceed stack[ACALC_STACKSIZE-1]. Valid stack depths are
unchanged (an expression that legitimately reached the old limit was
already writing out of bounds).

Test

Adds two acalcTest cases around NDERIV: 15 levels of nesting evaluate
normally; 16 levels are rejected by the runtime guard (aCalcPerform
returns non-zero) instead of overflowing. Without the fix the 16-level
case writes out of bounds and terminates acalcTest with SIGSEGV; with
the fix the full suite passes (150/150).

The value stack is calloc(ACALC_STACKSIZE) with top = &stack[1], so the
last writable element is stack[ACALC_STACKSIZE-1].  INC() advanced ps,
then guarded with `(ps-top) > ACALC_STACKSIZE` before writing (ps)->numEl
and (ps)->sourceDouble.  Since ps addresses stack[1 + (ps-top)], the write
lands out of bounds once (ps-top) reaches ACALC_STACKSIZE-1: the guard let
indices ACALC_STACKSIZE and ACALC_STACKSIZE+1 be written before tripping.

The postfix compiler caps a purely scalar expression at ACALC_STACKSIZE,
so scalar depth alone never reaches the guard.  NDERIV/DERIV/FITPOLY,
however, push two or three scratch elements at run time (aCalcPerform.c
NDERIV does DEC then three INCs) that the compiler's per-opcode
`runtime_effect` accounting models only as a net delta, not the transient
peak.  A deep-enough expression such as
`1+(1+(...+(NDERIV(AA,2))...))` therefore passes aCalcPostfix() yet drives
the runtime stack past its end, writing out of bounds and crashing the IOC.

Guard on `(ps-top) > ACALC_STACKSIZE-2` so INC() rejects before the write
can exceed stack[ACALC_STACKSIZE-1].  Valid depths are unchanged.

Add acalcTest cases: 15 levels of nesting around NDERIV evaluate; 16
levels are rejected by the runtime guard.  Without the fix the k=16 case
writes out of bounds and terminates acalcTest with SIGSEGV.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant