Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stack-based buffer overflow in jq master branch #3246

Closed
ShangzhiXu opened this issue Feb 3, 2025 · 1 comment
Closed

Stack-based buffer overflow in jq master branch #3246

ShangzhiXu opened this issue Feb 3, 2025 · 1 comment
Labels

Comments

@ShangzhiXu
Copy link

Describe the bug
An insufficient fix for CVE-2023-50268 has led to a stack-buffer-overflow in the master branch of jq.

To Reproduce
Hi team, thanks for your great work! While analyzing CVE-2023-50268, I discovered that the vulnerability has not been fully addressed. Although commit c9a5156 attempted to fix the issue by introducing a decNumberUnit member in res to prevent overflow when handling large numbers, this fix is insufficient. The decNumberUnit can only store a limited range of values, leaving relatively large numbers still capable of crashing the program.

To reproduce the issue, compile the master branch with the following configuration:
./configure CFLAGS="-g -O0 -fno-inline -rdynamic -fsanitize=address" CXXFLAGS="-g -O0 -fno-inline -rdynamic -fsanitize=address" LDFLAGS="-fsanitize=address"

and do

./jq '1 != .' <<<Nan100000000000000000000000000

you will see

==266385==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffe4ea915e0 at pc 0x7fa3b0b8b819 bp 0x7ffe4ea91380 sp 0x7ffe4ea91378
WRITE of size 2 at 0x7ffe4ea915e0 thread T0
    #0 0x7fa3b0b8b818 in decNumberCopy src/decNumber/decNumber.c:3375
    #1 0x7fa3b0ba24e5 in decNaNs src/decNumber/decNumber.c:7706
    #2 0x7fa3b0b9a979 in decCompareOp src/decNumber/decNumber.c:6085
    #3 0x7fa3b0b7d7da in decNumberCompare src/decNumber/decNumber.c:858
    #4 0x7fa3b0b46e11 in jvp_number_cmp src/jv.c:748
    #5 0x7fa3b0b47040 in jvp_number_equal src/jv.c:773
    #6 0x7fa3b0b5256f in jv_equal src/jv.c:1922
    #7 0x7fa3b0b18bc8 in binop_notequal src/builtin.c:409
    #8 0x7fa3b0b10721 in f_notequal src/builtin.c:57
    #9 0x7fa3b0b3c8cd in jq_next src/execute.c:919
    #10 0x5606c9c0604c in process src/main.c:180
    #11 0x5606c9c0947a in main src/main.c:666
    #12 0x7fa3b0855249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #13 0x7fa3b0855304 in __libc_start_main_impl ../csu/libc-start.c:360
    #14 0x5606c9c05590 in _start

Address 0x7ffe4ea915e0 is located in stack of thread T0 at offset 48 in frame
    #0 0x7fa3b0b46b90 in jvp_number_cmp src/jv.c:737

  This frame has 3 object(s):
    [32, 48) 'res' (line 746) <== Memory access at offset 48 overflows this variable
    [64, 80) 'a' (line 737)
    [96, 112) 'b' (line 737)
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow src/decNumber/decNumber.c:3375 in decNumberCopy
Shadow bytes around the buggy address:
  0x100049d4a260: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100049d4a270: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100049d4a280: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1
  0x100049d4a290: f1 f1 04 f3 f3 f3 00 00 00 00 00 00 00 00 00 00
  0x100049d4a2a0: f1 f1 f1 f1 04 f3 f3 f3 00 00 00 00 00 00 00 00
=>0x100049d4a2b0: 00 00 00 00 00 00 f1 f1 f1 f1 00 00[f2]f2 00 00
  0x100049d4a2c0: f2 f2 00 00 f3 f3 00 00 00 00 00 00 00 00 00 00
  0x100049d4a2d0: 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00
  0x100049d4a2e0: f2 f2 00 00 f3 f3 00 00 00 00 00 00 00 00 00 00
  0x100049d4a2f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100049d4a300: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==266385==ABORTING

Expected behavior
I think it should not crash here, as it can influence many bytes depending on the input number.

What I can see is that jq uses two members (each 2 bytes) to hold relatively large numbers that have more than three digits.
For example, 400 will be stored in one member, and in memory, it looks like 0x90 0x01, as 0x90 = 144 and 0x01 = 1, so:
1 × 256 + 144 = 400

For 4000, one member is not enough, so it is stored as 0x00 0x00 0x04 0x00, which means the lowest three digits are 000, and the first one is 4, adding up to 4000. As we can see, it should be stored in two members (4 bytes), as shown in the source code:

    struct {
      decNumber number;
      decNumberUnit units[1]; // added when fixing CVE-2023-50268 
    } res;

However, when number get tooooo large, like 10000000000000000000, two members would also be insufficient to hold it, leading to a buffer overflow. Maybe we should add a filter or dynamically allocate the variable res. 😊

Thanks!
Environment (please complete the following information):

  • OS and Version: [Debian GNU/Linux 12 ]
  • jq version [master]
@itchyny itchyny added the dup label Feb 3, 2025
@itchyny
Copy link
Contributor

itchyny commented Feb 3, 2025

Closing as duplicate issue of #3196 (and private security advisories).

@itchyny itchyny closed this as completed Feb 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants