stack-buffer-overflow in decNumber

[ChudaykinAlex]

decNumber — follow-up fuzzing fixes (OOB reads + UB), part 1: patch

This is an update to the previous decNumber fix (the three
stack-buffer-overflow cases in decFloatAdd / decFinalize). It extends that
work with a new batch of defects found by continued ASan+UBSan fuzzing of the same
routines. The earlier patch stays in place; this PR only adds the fixes below.

Background

Follow-up fuzz testing of the decimal floating-point routines in the decNumber
library — this time with ASan and UBSan together — surfaced a further batch of
defects beyond the three stack-buffer-overflow cases fixed earlier. They fall in
two classes:

• Out-of-bounds stack reads (stack-buffer-overflow, READ of size 4) in the
  NaN-payload comparison and in the FMA leading-zero strip.
• Undefined behaviour — signed integer overflow and signed left shift — while
  parsing over-long exponents and in the power/ln bit-walk loops.

Eight distinct defects are addressed here; a ninth reproduced scenario
(decQuadAdd, decBasic.c:1401) is already fixed by the previous PR and is kept
only as a regression case.

Potentially on versions 3, 4, 5, and master.

---

Affected files

File	Line	Function	Class
lib/decBasic.c	1190	decFloatAdd / decQuadAdd	signed left shift (UB)
lib/decBasic.c	1762	decQuadCompareTotal	OOB stack read
lib/decBasic.c	2245 (+1536, 2080, 2082)	decQuadFMA / strip-leading-zeros	OOB stack read
lib/decCommon.c	845	decFloatFromString / decQuadFromString	signed int overflow
lib/decNumber.c	598	decNumberFromString	signed overflow / left shift (UB)
lib/decNumber.c	2202	decNumberPower	signed left shift (UB)
lib/decNumber.c	5469	decNumberLn	signed left shift (UB)

---

How to reproduce

The reproducer new_poc.c is a deterministic dispatcher: seed_bugN[0] selects the
rounding mode, seed_bugN[1] % 16 selects the operation, and the remaining bytes are
split into two strings fed to the decQuad* / decNumber* functions.

GCC

gcc -g -O0 -fsanitize=address,undefined -fno-sanitize-recover=undefined -I./ \
    new_poc.c decContext.c decNumber.c decDouble.c decQuad.c decPacked.c -o poc_runner
./poc_runner 1   # 1..9

Clang

clang -g -O0 -fsanitize=address,undefined -fno-sanitize-recover=undefined -I./ \
    new_poc.c decContext.c decNumber.c decDouble.c decQuad.c decPacked.c -o poc_runner
./poc_runner 1   # 1..9

All cases fire consistently on the vulnerable code under ASan/UBSan with both compilers.

Previous reproducer (poc.c, regression check)

Since this is an update, the earlier three cases (decQuadAdd, decDoubleReduce,
decQuadReduce) should still pass. Build and run the original PoC the same way:

gcc -g -O0 -fsanitize=address,undefined -fno-sanitize-recover=undefined -I./ \
    decContext.c decNumber.c decDouble.c decQuad.c \
    decimal32.c decimal64.c decimal128.c decPacked.c poc.c -o poc_runner
./poc_runner 1   # 1..3

# clang: replace gcc with clang, same flags

new_poc.c
poc.c

---

Sanitizer output

Bug #1 — decQuadAdd (UBSan)

decBasic.c:1190:30: runtime error: left shift of 25 by 30 places cannot be represented in type 'int'

Bug #3 — decQuadCompareTotal (ASan)

ERROR: AddressSanitizer: stack-buffer-overflow ... READ of size 4
    #0 decQuadCompareTotal   decBasic.c:1762
[64, 102) 'bufl' (line 1745) <== Memory access at offset 99 partially overflows this variable

Bug #4 — decQuadFMA (ASan)

ERROR: AddressSanitizer: stack-buffer-overflow ... READ of size 4
    #0 decQuadFMA   decBasic.c:2245
[304, 416) 'acc' (line 2004) <== Memory access at offset 413 partially overflows this variable

Bug #5 — decQuadFromString (UBSan)

decCommon.c:845:20: runtime error: signed integer overflow: 999999999 * 10 cannot be represented in type 'int'

Bug #6 / #7 — decNumberFromString (UBSan)

decNumber.c:598:18: runtime error: left shift of 999999999 by 3 places cannot be represented in type 'int'
decNumber.c:598:18: runtime error: signed integer overflow: 444444444 + 1777777776 cannot be represented in type 'int'

Bug #8 — decNumberPower (UBSan)

decNumber.c:2202:12: runtime error: left shift of 1660944384 by 1 places cannot be represented in type 'int'

Bug #9 — decNumberLn (UBSan)

decNumber.c:5469:12: runtime error: left shift of 1342177280 by 1 places cannot be represented in type 'int'

---

Changes

lib/decBasic.c — bug #1 — decFloatAdd

BIN2DPD[] is uShort, promoted to signed int; a DPD value (up to 12 bits) shifted
by 20/26/28/30 places reaches the sign bit — UB. encode is uInt and the high-bit
truncation is intended, so shift in unsigned arithmetic:

// before
encode|=BIN2DPD[tac[2]]<<20;
encode|=BIN2DPD[tac[3]]<<30;
encode|=BIN2DPD[tac[6]]<<28;
encode|=BIN2DPD[tac[9]]<<26;
// after
encode|=(uInt)BIN2DPD[tac[2]]<<20;
encode|=(uInt)BIN2DPD[tac[3]]<<30;
encode|=(uInt)BIN2DPD[tac[6]]<<28;
encode|=(uInt)BIN2DPD[tac[9]]<<26;

lib/decBasic.c — bug #3 — decQuadCompareTotal

When the byte-wise inner loop found a differing NaN-payload byte it broke only out of
the inner loop; the outer 4-byte loop then continued from a misaligned pointer and read
past bufl. It was also a logic bug (the winner must be the first/most-significant
difference). Break out of the outer loop once a winner is found:

    for (;; ub++, uc++) {
      if (*ub==*uc) continue;
      if (*ub>*uc) comp=sigl;
       else comp=-sigl;
       break;
      }
    break;                               // winner found; stop scanning
    }

lib/decBasic.c — bug #4 — strip-leading-zeros loops (lines 1536, 2080, 2082, 2245)

&& evaluates the 4-byte UBTOUI() read before the bounds check, reading past the
coefficient near its end. Test the bound first (matching decCommon.c:267):

// before
for (; UBTOUI(x)==0 && x+3<lsd;) x+=4;
// after
for (; x+3<lsd && UBTOUI(x)==0;) x+=4;

lib/decCommon.c — bug #5 — decFloatFromString

The "exponent too long" clip runs after the accumulation loop, so exp*10 overflows
int inside the loop. Accumulate unsigned (the over-large value is still clipped):

// before
exp=exp*10+edig;
// after
exp=(Int)((uInt)exp*10+edig);

lib/decNumber.c — bugs #6, #7 — decNumberFromString

X10(exponent) on a long exponent both left-shifts into the sign bit and overflows the
add. Legitimate 10-digit exponents (up to 1999999999) must be preserved, so compute
X10 unsigned; genuinely oversized exponents are still cut by the existing length/first
-digit clip:

// before
exponent=X10(exponent)+(Int)*c-(Int)'0';
// after
exponent=(Int)(X10((uInt)exponent)+(uInt)((Int)*c-(Int)'0'));

lib/decNumber.c — bugs #8, #9 — decNumberPower (2202) and decNumberLn (5469)

The exponent bit-walk shifts a signed Int left while testing its sign — UB when a bit
reaches the sign position. Shift in unsigned and reinterpret (the n<0 sign test is
preserved):

// before
n=n<<1;
// after
n=(Int)((uInt)n<<1);

---

Verification

Rebuilt poc_runner with the same command and ran cases 1–9 under ASan+UBSan
(-fno-sanitize-recover=undefined):

=== BUG 1 === clean        === BUG 6 === clean
=== BUG 2 === clean        === BUG 7 === clean
=== BUG 3 === clean        === BUG 8 === clean
=== BUG 4 === clean        === BUG 9 === clean
=== BUG 5 === clean

All nine exit cleanly with no ASan/UBSan diagnostic, on both clang and gcc. The previous
reproducer poc.c (cases 1–3) also stays clean, confirming no regression of the earlier
fix:

=== poc.c CASE 1 === clean
=== poc.c CASE 2 === clean
=== poc.c CASE 3 === clean

Functional sanity checks pass unchanged (decQuadAdd, decQuadCompareTotal,
decQuadFMA, decNumberFromString, decNumberPower, decNumberLn), confirming the
fixes preserve arithmetic behaviour.

[ChudaykinAlex]

@AlexPeshkoff Good day. Should I write a unit test?

[AlexPeshkoff]

Should I write a unit test?

If you wish - please do. On the one hand result appears to be obvious, but additional test makes no harm.

[ChudaykinAlex]

Question: How do I run the decNumber regression tests so that they actually catch bugs?

I added regression test cases for the decNumber defect batch in src/common/tests/DecNumberTest.cpp (on POSIX, these are automatically picked up by the common_test target; on MSVC, you need to manually add them to common_test.vcxproj). The problem is that most of these tests only catch regressions when built with sanitizers, and I don’t see any options for such a build in the project tree.

The defects fall into two categories:

• Out-of-bounds stack reads (decQuadCompareTotal, decQuadFMA) — these consistently fail only under ASan. Without it, they usually read a few bytes beyond the buffer boundary and continue running silently.
• Undefined behavior—signed integer overflow / signed left shift (decFloatAdd, decFloatFromString, decNumberFromString, decNumberPower, decNumberLn)—is diagnosed only under UBSan (-fsanitize=undefined -fno-sanitize-recover=undefined). On a standard two’s-complement build, a “wrapped” value is exactly what the code would clip anyway, so the result is the same with or without the fix: the test passes in either case and catches nothing.

So, with a regular make run_tests, these cases effectively only verify that the program “doesn’t crash,” which is meaningless for UB cases.

There are two options, but in my opinion, they are unsatisfactory.

1. Introduce a configure flag (for example, --enable-asan), which adds -fsanitize=address,undefined -fno-sanitize-recover=undefined to CFLAGS/CXXFLAGS/LDFLAGS in builds/posix/make.defaults, and set up a separate CI job that builds and runs common_test under this profile. In a regular build, the tests remain simple “it works/the result is correct” checks, while the sanitizer run provides actual protection against bugs.
2. To allow the sanitizers to detect a bug within the library itself, pass the same flags to the nested make extern/decNumber—pass them to CFLAGS when calling `$ (MAKE) -C .../extern/decNumber in builds/posix/Makefile.in (currently, the library is hard-coded to build with -O3 in its Makefile). Then libdecFloat.a is compiled with instrumentation under the sanitizer profile, and UBSan/ASan trigger in decNumber.