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

OOB Read memory corruption bug #548

Closed
JordyZomer opened this issue Sep 1, 2020 · 9 comments
Closed

OOB Read memory corruption bug #548

JordyZomer opened this issue Sep 1, 2020 · 9 comments

Comments

@JordyZomer
Copy link

JordyZomer commented Sep 1, 2020

Hi,

I encountered an OOB read memory corruption bug when fuzzing Jansson.

Below you can find the crash log:

# ./prog -detect_leaks=0
INFO: Seed: 665849601
INFO: Loaded 1 modules   (10 inline 8-bit counters): 10 [0x5a3f90, 0x5a3f9a),
INFO: Loaded 1 PC tables (10 PCs): 10 [0x568020,0x5680c0),
INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes
INFO: A corpus is not provided, starting from an empty corpus
#2      INITED cov: 3 ft: 4 corp: 1/1b exec/s: 0 rss: 27Mb
#19711  NEW    cov: 6 ft: 7 corp: 2/4b lim: 198 exec/s: 0 rss: 47Mb L: 3/3 MS: 4 InsertByte-InsertByte-ShuffleBytes-ShuffleBytes-
#19777  REDUCE cov: 6 ft: 7 corp: 2/3b lim: 198 exec/s: 0 rss: 47Mb L: 2/2 MS: 1 EraseBytes-
#524288 pulse  cov: 6 ft: 7 corp: 2/3b lim: 4096 exec/s: 262144 rss: 610Mb
#1048576        pulse  cov: 6 ft: 7 corp: 2/3b lim: 4096 exec/s: 262144 rss: 658Mb
#2097152        pulse  cov: 6 ft: 7 corp: 2/3b lim: 4096 exec/s: 262144 rss: 659Mb
#4194304        pulse  cov: 6 ft: 7 corp: 2/3b lim: 4096 exec/s: 262144 rss: 659Mb
#8388608        pulse  cov: 6 ft: 7 corp: 2/3b lim: 4096 exec/s: 262144 rss: 659Mb
#16777216       pulse  cov: 6 ft: 7 corp: 2/3b lim: 4096 exec/s: 254200 rss: 659Mb
#33554432       pulse  cov: 6 ft: 7 corp: 2/3b lim: 4096 exec/s: 252288 rss: 659Mb
#67108864       pulse  cov: 6 ft: 7 corp: 2/3b lim: 4096 exec/s: 246723 rss: 659Mb
#134217728      pulse  cov: 6 ft: 7 corp: 2/3b lim: 4096 exec/s: 241833 rss: 659Mb
AddressSanitizer:DEADLYSIGNAL
=================================================================
==1898031==ERROR: AddressSanitizer: SEGV on unknown address 0x610000050000 (pc 0x7f229f4f020b bp 0x7ffcf5010a30 sp 0x7ffcf5010938 T0)
==1898031==The signal is caused by a READ memory access.
    #0 0x7f229f4f020b in string_get /root/jansson-2.13/src/load.c:893:7
    #1 0x7f229f4f06f3 in stream_get.part.0 /root/jansson-2.13/src/load.c:158:13
    #2 0x7f229f4f0837 in stream_get /root/jansson-2.13/src/load.c:154:8
    #3 0x7f229f4f0837 in lex_get_save /root/jansson-2.13/src/load.c:233:13
    #4 0x7f229f4f0a32 in lex_scan /root/jansson-2.13/src/load.c:604:17
    #5 0x7f229f4f15da in parse_json.constprop.0 /root/jansson-2.13/src/load.c:868:9
    #6 0x7f229f4f1721 in json_loads /root/jansson-2.13/src/load.c:920:14
    #7 0x54dad3 in LLVMFuzzerTestOneInput /root/fuzzing/jansson/fuzz.c:12:9
    #8 0x4586a1 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/root/fuzzing/jansson/prog+0x4586a1)
    #9 0x457de5 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) (/root/fuzzing/jansson/prog+0x457de5)
    #10 0x45a087 in fuzzer::Fuzzer::MutateAndTestOne() (/root/fuzzing/jansson/prog+0x45a087)
    #11 0x45ad85 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) (/root/fuzzing/jansson/prog+0x45ad85)
    #12 0x44973e in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/root/fuzzing/jansson/prog+0x44973e)
    #13 0x472582 in main (/root/fuzzing/jansson/prog+0x472582)                                                                                                                                                  #14 0x7f229f1810b2 in __libc_start_main /build/glibc-YYA7BZ/glibc-2.31/csu/../csu/libc-start.c:308:16
    #15 0x41e4dd in _start (/root/fuzzing/jansson/prog+0x41e4dd)
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /root/jansson-2.13/src/load.c:893:7 in string_get
==1898031==ABORTING
MS: 2 InsertRepeatedBytes-CopyPart-; base unit: 97d170e1550eee4afc0af065b78cda302a97674c
0x5b,0x5d,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67
,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x6
7,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x
67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0
x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,0x67,
[]gggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggg
artifact_prefix='./'; Test unit written to ./crash-76b9ab07b593b8511d24aea00b925d60735e9d6c
Base64: W11nZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dn
Z2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dnZ2dn

The fuzzer I used was:

#include <jansson.h>
#include <stdio.h>

int LLVMFuzzerTestOneInput(const char *Data, size_t Size) {
        FILE *devNull;

        devNull = fopen("/dev/null", "w");

        json_t *root;
        json_error_t error;

        root = json_loads(Data, 0, &error);

        json_dumpf(root, devNull, 0);
        json_decref(root);

        fclose(devNull);

        return 0;
}

Kind Regards,

Jordy Zomer

@carnil
Copy link

carnil commented Apr 26, 2021

This issue appears to have been assigned CVE-2020-36325.

@DavidKorczynski
Copy link

DavidKorczynski commented Apr 27, 2021

This is not a bug. json_loads expects a null-terminated string - the above fuzzer is invalid as it does not ensure proper null-termination. This is how a lot of C functions operate: think strlen - or basically any other function in string.h - you will find OOBs all over libc if you don't provide null-terminated strings.

If this was really assigned a CVE I recon it should be retracted.

@akheron
Copy link
Owner

akheron commented Apr 28, 2021

Thanks for the analysis @DavidKorczynski

@akheron akheron closed this as completed Apr 28, 2021
@carnil
Copy link

carnil commented Apr 28, 2021 via email

@carnil
Copy link

carnil commented Apr 28, 2021

@DavidKorczynski FTR it got not REJECTED but now marked as "disputed", cf. https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-36325

@DavidKorczynski
Copy link

DavidKorczynski commented Apr 28, 2021

Interesting, thanks for the update! I wonder how this plays out - I would think the CVE team must have some means of handling false positives in particular with memory unsafe languages and libraries.

@DavidKorczynski
Copy link

DavidKorczynski commented Apr 28, 2021

For reference "When one party disagrees with another party's assertion that a particular issue in software is a vulnerability, a CVE Record assigned to that issue may be designated as being "DISPUTED". In these cases, the CVE Program is making no determination as to which party is correct." from here https://cve.mitre.org/about/faqs.html#disputed_signify_in_cve_record

However, in this case it seems rejected is the correct classification:
"A CVE Record listed as "REJECT" is a CVE Record that is not accepted as a CVE Record. The reason a CVE Record is marked REJECT will most often be stated in the description of the CVE Record. Possible examples include it being a duplicate CVE Record, it being withdrawn by the original requester, it being assigned incorrectly, or some other administrative reason." from the same URL.

@DavidKorczynski
Copy link

Anyways - issue is closed and it's no big deal.

algitbot pushed a commit to alpinelinux/aports that referenced this issue May 4, 2021
Upstream notes this is not a valid real-world bug.
akheron/jansson#548 (comment)
halstead pushed a commit to openembedded/meta-openembedded that referenced this issue Dec 5, 2021
According to the upstream [1], the bug happens only if the programmer
does not follow the API definition.

[1] akheron/jansson#548

Signed-off-by: Marta Rybczynska <[email protected]>
Signed-off-by: Armin Kuster <[email protected]>
jpuhlman pushed a commit to MontaVista-OpenSourceTechnology/meta-openembedded that referenced this issue Dec 7, 2021
Source: meta-openembedded
MR: 113447
Type: Integration
Disposition: Merged from meta-openembedded
ChangeID: e0e79bb
Description:

According to the upstream [1], the bug happens only if the programmer
does not follow the API definition.

[1] akheron/jansson#548

Signed-off-by: Marta Rybczynska <[email protected]>
Signed-off-by: Armin Kuster <[email protected]>
Signed-off-by: Jeremy Puhlman <[email protected]>
jpuhlman pushed a commit to MontaVista-OpenSourceTechnology/meta-openembedded that referenced this issue Dec 9, 2021
Source: meta-openembedded
MR: 113447
Type: Integration
Disposition: Merged from meta-openembedded
ChangeID: e0e79bb
Description:

According to the upstream [1], the bug happens only if the programmer
does not follow the API definition.

[1] akheron/jansson#548

Signed-off-by: Marta Rybczynska <[email protected]>
Signed-off-by: Armin Kuster <[email protected]>
Signed-off-by: Jeremy Puhlman <[email protected]>
@vtamara
Copy link

vtamara commented Aug 1, 2022

It is well known that functions like strlen that depend on null-terminated strings exclusively are not safe. But what about improving the API of jansson? json_loads could receive a fourth parameter with the maximal length of the parameter data.

halstead pushed a commit to openembedded/meta-openembedded that referenced this issue Apr 5, 2023
According to the upstream [1], the bug happens only if the programmer
does not follow the API definition.

[1] akheron/jansson#548

Signed-off-by: Marta Rybczynska <[email protected]>
Signed-off-by: Armin Kuster <[email protected]>
(cherry picked from commit e0e79bb)
[Fixup for Kirkstone context]
Signed-off-by: Armin Kuster <[email protected]>
amstewart pushed a commit to ni/meta-openembedded that referenced this issue Apr 7, 2023
According to the upstream [1], the bug happens only if the programmer
does not follow the API definition.

[1] akheron/jansson#548

Signed-off-by: Marta Rybczynska <[email protected]>
Signed-off-by: Armin Kuster <[email protected]>
(cherry picked from commit e0e79bb)
[Fixup for Kirkstone context]
Signed-off-by: Armin Kuster <[email protected]>
jpuhlman pushed a commit to MontaVista-OpenSourceTechnology/meta-openembedded that referenced this issue Apr 10, 2023
Source: meta-openembedded
MR: 124675
Type: Integration
Disposition: Merged from meta-openembedded
ChangeID: 96bd928
Description:

According to the upstream [1], the bug happens only if the programmer
does not follow the API definition.

[1] akheron/jansson#548

Signed-off-by: Marta Rybczynska <[email protected]>
Signed-off-by: Armin Kuster <[email protected]>
(cherry picked from commit e0e79bb)
[Fixup for Kirkstone context]
Signed-off-by: Armin Kuster <[email protected]>
Signed-off-by: Jeremy A. Puhlman <[email protected]>
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

No branches or pull requests

5 participants