LibFuzzer: STUN crash [@nr_stun_decode_message]

RESOLVED FIXED in Firefox 62

Status

()

defect
P2
critical
Rank:
15
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: posidron, Assigned: dminor)

Tracking

({crash, testcase})

Trunk
mozilla62
x86_64
Linux
Points:
---

Firefox Tracking Flags

(firefox-esr52 wontfix, firefox-esr60 wontfix, firefox60 wontfix, firefox61 wontfix, firefox62 fixed)

Details

(crash signature)

Attachments

(2 attachments, 1 obsolete attachment)

Reporter

Description

a year ago
The following testcase crashes on mozilla-central revision 20180523-d36cd8bdbc5c

See attachment.

Backtrace:

==34==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000808 (pc 0x7f25d7b97472 bp 0x7ffde57d9f50 sp 0x7ffde57d9e00 T0)
==34==The signal is caused by a READ memory access.
==34==Hint: address points to the zero page.
SCARINESS: 10 (null-deref)
    #0 0x7f25d7b97471 in nr_stun_decode_message /builds/worker/workspace/build/src/media/mtransport/third_party/nICEr/src/stun/stun_codec.c:1418:11
    #1 0x7f25d8c0ebe5 in RunStunParserFuzzing(unsigned char const*, unsigned long) /builds/worker/workspace/build/src/media/mtransport/fuzztest/stun_parser_libfuzz.cpp:31:3
    #2 0x5893fd in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /builds/worker/workspace/build/src/tools/fuzzing/libfuzzer/FuzzerLoop.cpp:517:13
    #3 0x588c7b in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) /builds/worker/workspace/build/src/tools/fuzzing/libfuzzer/FuzzerLoop.cpp:442:3
    #4 0x58a16d in fuzzer::Fuzzer::MutateAndTestOne() /builds/worker/workspace/build/src/tools/fuzzing/libfuzzer/FuzzerLoop.cpp:650:19
    #5 0x58ab25 in fuzzer::Fuzzer::Loop(std::vector<std::string, fuzzer::fuzzer_allocator<std::string> > const&) /builds/worker/workspace/build/src/tools/fuzzing/libfuzzer/FuzzerLoop.cpp:773:5
    #6 0x582185 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /builds/worker/workspace/build/src/tools/fuzzing/libfuzzer/FuzzerDriver.cpp:754:6
    #7 0x7f25d7af81e5 in mozilla::FuzzerRunner::Run(int*, char***) /builds/worker/workspace/build/src/tools/fuzzing/interface/harness/FuzzerRunner.cpp:60:10
    #8 0x7f25d7a37b33 in XREMain::XRE_mainStartup(bool*) /builds/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:3916:35
    #9 0x7f25d7a48f45 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4857:12
    #10 0x7f25d7a4a6d4 in XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4964:21
    #11 0x4f4ef5 in do_main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:233:22
    #12 0x4f4ef5 in main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:306
    #13 0x7f25ef20a1c0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x211c0)
    #14 0x42476c in _start (/home/worker/firefox/firefox+0x42476c)

DEDUP_TOKEN: nr_stun_decode_message
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /builds/worker/workspace/build/src/media/mtransport/third_party/nICEr/src/stun/stun_codec.c:1418:11 in nr_stun_decode_message

Command: ./firefox/firefox -print_pcs=1 -handle_segv=0 -handle_bus=0 -handle_abrt=0 ./corpora/ -handle_ill=0 -handle_fpe=0

==34==ABORTING
Reporter

Comment 1

a year ago
Posted file Testcase
Assignee

Updated

a year ago
Assignee: nobody → dminor
Status: NEW → ASSIGNED
Rank: 15
Priority: -- → P2
Comment hidden (mozreview-request)

Comment 3

a year ago
mozreview-review
Comment on attachment 8980643 [details]
Bug 1464069 - Check for null message in nr_stun_decode_message;

https://reviewboard.mozilla.org/r/246810/#review253026

LGTM
Attachment #8980643 - Flags: review?(drno) → review+

Comment 4

a year ago
Pushed by dminor@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8ea04450a354
Check for null message in nr_stun_decode_message; r=drno
I'm trying to understand how this happens. Part of the API contract to this function is that you pass it a valid pointer. So, one of two things is happening here:

1. You're directly violating the contract, in which case this isn't a valid test case.
2. Some other nICEr function is calling it with a null pointer, in which case that function has a bug.

Based on the call stack, the problem looks like 1, in which case, this change is unnecessary.
Flags: needinfo?(cdiehl)

Comment 6

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8ea04450a354
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Reporter

Comment 7

a year ago
(In reply to Eric Rescorla (:ekr) from comment #5)
> I'm trying to understand how this happens. Part of the API contract to this
> function is that you pass it a valid pointer. So, one of two things is
> happening here:
> 
> 1. You're directly violating the contract, in which case this isn't a valid
> test case.
> 2. Some other nICEr function is calling it with a null pointer, in which
> case that function has a bug.
> 
> Based on the call stack, the problem looks like 1, in which case, this
> change is unnecessary.


It was found by calling [1] in the STUN libfuzzer module and by feeding it these dumped STUN corpora [2].

[1] https://searchfox.org/mozilla-central/source/media/mtransport/fuzztest/stun_parser_libfuzz.cpp#31
[2] https://github.com/MozillaSecurity/fuzzdata/tree/master/samples/stun
Flags: needinfo?(cdiehl)
This test case doesn't seem quite right. Here's nr_stun_message_create2() https://searchfox.org/mozilla-central/source/media/mtransport/third_party/nICEr/src/stun/stun_msg.c#53. The prototype is:

int nr_stun_message_create2(nr_stun_message **msg, UCHAR *buffer, size_t length);

The only real ways it can fail are:

1. Out of memory
2. You pass it a too large message (|length| > NR_STUN_MAX_MESSAGE_SIZE)

I'm assuming you are doing the second. In either case, it leaves |*msg| unset (you have it initialized to NULL) and returns a nonzero error code. In order to use this API correctly, you need to check the error code prior to calling nr_stun_decode_message(). Here is an example from the nICEr code.

https://searchfox.org/mozilla-central/source/media/mtransport/third_party/nICEr/src/stun/stun_client_ctx.c#565

In general, (nearly) every nICEr function returns an error value, and you can't use its outputs if that error is nonzero.
Dan, looks like we fixed the wrong end here. Can you fix the fuzzing client please?
Flags: needinfo?(dminor)
Assignee

Comment 10

a year ago
Sure thing.
Status: RESOLVED → REOPENED
Flags: needinfo?(dminor)
Resolution: FIXED → ---
Assignee

Comment 11

a year ago
Attachment #8980643 - Attachment is obsolete: true
Attachment #8981563 - Flags: review?(drno)
Assignee

Comment 12

a year ago
Sorry about this, I saw what looked like a quick fix while doing triage. In the future, I'll leave these alone unless I'm familiar with the code involved.
Comment on attachment 8981563 [details] [diff] [review]
bug-1464069-fix.patch

Review of attachment 8981563 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM
Attachment #8981563 - Flags: review?(drno) → review+

Comment 14

a year ago
Pushed by dminor@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/66c01f311f4a
Check error code in stun_parser_libfuzz.cpp; r=drno

Comment 15

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/66c01f311f4a
Status: REOPENED → RESOLVED
Last Resolved: a year agoa year ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.