Closed Bug 1037718 Opened 6 years ago Closed 5 years ago

AddressSanitizer: global-buffer-overflow [@ JS_GetTypeName] with Symbol

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla34
Tracking Status
firefox32 --- unaffected
firefox33 --- disabled
firefox34 --- verified
firefox-esr24 --- unaffected
firefox-esr31 --- unaffected
b2g-v1.3 --- unaffected
b2g-v1.3T --- unaffected
b2g-v1.4 --- unaffected
b2g-v2.0 --- unaffected
b2g-v2.1 --- fixed

People

(Reporter: decoder, Assigned: jorendorff)

References

(Blocks 2 open bugs)

Details

(Keywords: coverity, sec-moderate, testcase, Whiteboard: [CID 1225481])

Attachments

(1 file)

The following testcase shows a global-buffer-overflow on mozilla-central revision e1a037c085d1 (run with --fuzzing-safe):


parse(Symbol());
ASan trace:

=================================================================
==15228==ERROR: AddressSanitizer: global-buffer-overflow on address 0x00000242aed8 at pc 0xe10925 bp 0x7fff1e381cc0 sp 0x7fff1e381cb8
READ of size 8 at 0x00000242aed8 thread T0
    #0 0xe10924 in JS_GetTypeName(JSContext*, JSType) js/src/jsapi.cpp:450
    #1 0x4a0f7f in Parse(JSContext*, unsigned int, JS::Value*) js/src/shell/js.cpp:3579
    #2 0x11dd843 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) js/src/jscntxtinlines.h:230
    #3 0x11dd843 in js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) js/src/vm/Interpreter.cpp:454
    #4 0x11d0631 in Interpret(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:2558
    #5 0x11b41a3 in js::RunScript(JSContext*, js::RunState&) js/src/vm/Interpreter.cpp:408
    #6 0x11a5d82 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::ExecuteType, js::AbstractFramePtr, JS::Value*) js/src/vm/Interpreter.cpp:616
    #7 0x11df97e in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) js/src/vm/Interpreter.cpp:652
    #8 0xe449db in ExecuteScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::Value*) js/src/jsapi.cpp:4796
    #9 0x48c0f4 in RunFile(JSContext*, JS::Handle<JSObject*>, char const*, _IO_FILE*, bool) js/src/shell/js.cpp:450
    #10 0x48c0f4 in Process(JSContext*, JSObject*, char const*, bool) js/src/shell/js.cpp:583
    #11 0x484a6b in ProcessArgs(JSContext*, JSObject*, js::cli::OptionParser*) js/src/shell/js.cpp:5957
    #12 0x484a6b in Shell(JSContext*, js::cli::OptionParser*, char**) js/src/shell/js.cpp:6171
    #13 0x484a6b in main js/src/shell/js.cpp:6460
    #14 0x7ff248ce176c in __libc_start_main /build/buildd/eglibc-2.15/csu/libc-start.c:226
    #15 0x47a6a8 in _start (js/src/opt64asan/js/src/shell/js+0x47a6a8)

0x00000242aed8 is located 40 bytes to the left of global variable 'JSRuntime::initializeAtoms(JSContext*)::cachedNames' from 'js/src/jsatom.cpp' (0x242af00) of size 3968
0x00000242aed8 is located 0 bytes to the right of global variable 'js::TypeStrings' from 'js/src/jsatom.cpp' (0x242aea0) of size 56
SUMMARY: AddressSanitizer: global-buffer-overflow js/src/jsapi.cpp:450 JS_GetTypeName(JSContext*, JSType)
Shadow bytes around the buggy address:
  0x00008047d580: 00 00 00 00 00 00 00 00 00 00 00 00 f9 f9 f9 f9
  0x00008047d590: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 00 00 00 00
  0x00008047d5a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00008047d5b0: 00 00 00 00 00 00 00 00 00 00 00 00 f9 f9 f9 f9
  0x00008047d5c0: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
=>0x00008047d5d0: 00 00 00 00 00 00 00 00 00 00 00[f9]f9 f9 f9 f9
  0x00008047d5e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00008047d5f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00008047d600: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00008047d610: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00008047d620: 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
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1



Not sure if this has any relevance outside the shell, but marking s-s to be safe. Needinfo from jorendorff because it involves Symbol().
Flags: needinfo?(jorendorff)
Is this a bug in the shell-only parse(), or parse just exposing something that would be there regardless?
Assignee: nobody → jorendorff
Keywords: sec-moderate
Doesn't seem like JS_GetTypeName() is used many places.
It looks like there is an entry missing in the array TypeStrings.  The only place this seems to be accessed outside of the shell is in constructing an error message in the method js::DefaultValue().

Waldo, could you fix this, or suggest somebody else who could fix it?  It looks pretty simple to fix.  Maybe an intern wants to fix a sec bug? ;)
Flags: needinfo?(jwalden+bmo)
This is really just yet another followup to bug 645416, that properly belongs on jorendorff's plate, I think.  Symbols aren't enabled outside of nightlies yet, precisely because of these missing-all-value-types issues that are likely to pop up for some time.  So this seems like a relatively low-importance security issue, unless we're seriously concerned about attacks against nightly users.  (Which, it's conceivable we should be, but isn't how we've handled nightly-only things wrong with, say, PJS or parallel arrays or TypedObject and so on in the past.)
Flags: needinfo?(jwalden+bmo)
Ah, great, I didn't realize symbols were being kept on Nightly for now.  Thanks.
apparently it isn't nightly only after all, per IRC, until bug 1041631 lands.
How about we just get rid of it?
Attachment #8468880 - Flags: review?(jwalden+bmo)
Flags: needinfo?(jorendorff)
Attachment #8468880 - Flags: review?(jwalden+bmo) → review+
Duplicate of this bug: 1037890
Dang. Why didn't we backport this to 33 since it is marked as affected or is it 33 unaffected?
Flags: needinfo?(jorendorff)
Unaffected.

It was marked "status-ff33: affected" in comment 7, with the remark "until bug 1041631 lands". That bug landed. Changing it back.
Flags: needinfo?(jorendorff)
Reproduced the original issue using the m-c changeset e1a037c085d1 with the instructions in comment #0:

* ==4583==ERROR: AddressSanitizer: global-buffer-overflow on address 0x00000268bed8 at pc 0xfc1a35 bp 0x7fff9d9d2200 sp 0x7fff9d9d21f8

Went through verification using the following builds: (ran the poc 5 different times for each build)

- m-c (using changeset ced1402861b8): PASSED
- m-a (using changeset 7242a447378b): PASSED
- m-b (using changeset c8ff4c93ee85): PASSED
Status: RESOLVED → VERIFIED
Group: core-security
Coverity found it too.
Keywords: coverity
Whiteboard: [CID 1225481]
You need to log in before you can comment on or make changes to this bug.