Closed Bug 788701 Opened 13 years ago Closed 13 years ago

Invalid read of size 2 [@ str_contains] involving map

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla18
Tracking Status
firefox16 --- unaffected
firefox17 --- fixed
firefox18 --- fixed
firefox-esr10 --- unaffected

People

(Reporter: gkw, Assigned: Benjamin)

Details

(4 keywords, Whiteboard: [qa-])

Attachments

(1 file)

[,,8].map(String.prototype.contains,this) shows the following error on Valgrind in 32-bit js opt shell on m-c changeset ce16c39e01fb: ==463== Invalid read of size 2 ==463== at 0xE30FB: str_contains(JSContext*, unsigned int, JS::Value*) (jsstr.cpp:984) ==463== by 0x8E0BF: js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) (jscntxtinlines.h:372) ==463== by 0x2A235: array_map(JSContext*, unsigned int, JS::Value*) (jsinterp.h:119) ==463== by 0x8E0BF: js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) (jscntxtinlines.h:372) ==463== by 0x860E4: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (jsinterp.cpp:2405) ==463== by 0x7ADAB: js::RunScript(JSContext*, JSScript*, js::StackFrame*) (jsinterp.cpp:301) ==463== by 0x8E870: js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::ExecuteType, js::StackFrame*, JS::Value*) (jsinterp.cpp:486) ==463== by 0x8E9E2: js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) (jsinterp.cpp:523) ==463== by 0x1EC1C: JS_ExecuteScript (jsapi.cpp:5665) ==463== by 0x74CA: Process(JSContext*, JSObject*, char const*, bool) (js.cpp:433) ==463== by 0x6007: Shell(JSContext*, js::cli::OptionParser*, char**) (js.cpp:4738) ==463== by 0x681C: main (js.cpp:4950) ==463== Address 0x2749070 is 0 bytes after a block of size 32 alloc'd ==463== at 0x3D3785: malloc (vg_replace_malloc.c:274) ==463== by 0x1B0586: js::Vector<unsigned short, 32ul, js::ContextAllocPolicy>::extractRawBuffer() (Utility.h:153) ==463== by 0x1B020E: js::StringBuffer::finishString() (StringBuffer.cpp:21) ==463== by 0x9CAEA: js::obj_toStringHelper(JSContext*, JSObject*) (jsobj.cpp:302) ==463== by 0xA246A: obj_toString(JSContext*, unsigned int, JS::Value*) (jsobj.cpp:359) ==463== by 0x8E0BF: js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) (jscntxtinlines.h:372) ==463== by 0x8E346: js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::Value*) (jsinterp.h:119) ==463== by 0xAD1BF: js::DefaultValue(JSContext*, JS::Handle<JSObject*>, JSType, JS::MutableHandle<JS::Value>) (jsobj.cpp:4897) ==463== by 0xE7F21: js::ToStringSlow(JSContext*, JS::Value const&) (jsobjinlines.h:69) ==463== by 0xE2E0C: str_contains(JSContext*, unsigned int, JS::Value*) (jsapi.h:1467) ==463== by 0x8E0BF: js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) (jscntxtinlines.h:372) ==463== by 0x2A235: array_map(JSContext*, unsigned int, JS::Value*) (jsinterp.h:119) Command: valgrind --dsymutil=yes --leak-check=full --suppressions=/Users/fuzz3/fuzzing/known/mozilla-central/valgrind.txt ./js testcase.js May be related to bug 779025. s-s because this is an invalid read.
Assignee: general → jorendorff
Definitely not related. That's a different kind of Map.
Summary: Invalid read of size 2 [@ str_contains] involving Map → Invalid read of size 2 [@ str_contains] involving map
autoBisect.py --valgrind -a32 -p 788701.js (with some autoBisect tweaks) The first bad revision is: changeset: 87e7abe891a9 user: Benjamin Peterson date: Fri Aug 03 11:37:27 2012 -0700 summary: Bug 772733 - Implement String.contains. r=sfink
Could reproduce on 64-bits.
(In reply to Benjamin Peterson [:benjamin] from comment #3) > Could reproduce on 64-bits. Uh, rather, could not reproduce.
That was my experience as well. Let me know if you need help building 32-bit.
I need help. Is it possible to build 32-bit on Mac these days? What's documented doesn't work, because the current code no longer compiles under gcc-apple-4.2. https://developer.mozilla.org/en-US/docs/Developer_Guide/Build_Instructions/Mac_OS_X_Prerequisites
I append environment variables - tested on 10.7 and 10.8. LD=ld CROSS_COMPILE=1 CC="clang -Qunused-arguments -fcolor-diagnostics -arch i386" RANLIB=ranlib CXX="clang++ -Qunused-arguments -fcolor-diagnostics -arch i386" AS=$CC AR=ar STRIP="strip -x -S" HOST_CC="clang -Qunused-arguments -fcolor-diagnostics" HOST_CXX="clang++ -Qunused-arguments -fcolor-diagnostics" sh ./configure --target=i386-apple-darwin8.0.0 --enable-optimize --disable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --enable-more-deterministic --disable-tests --enable-valgrind (Switch to Clang?)
I switched to Clang a while ago. I guess we should switch the docs too. OK, now the problem I have is that valgrind won't run the resulting executable.
> OK, now the problem I have is that valgrind won't run the resulting > executable. Try compiling it manually, docs are at http://www.valgrind.org/downloads/repository.html
Or simply use Linux, the bug is not Mac specified.
I quickly looked into this in a debug build, and it seems that StringMatch is being called with a text pointer that is inside the "[Object global]" string, rather than at the start: Breakpoint 1, StringMatch (text=0x8600114, textlen=15, pat=0xf7500328, patlen=1) at js/src/jsstr.cpp:971 971 if (patlen == 0) (gdb) x /30bx 0x8600114 0x8600114: 0x62 0x00 0x6a 0x00 0x65 0x00 0x63 0x00 0x860011c: 0x74 0x00 0x20 0x00 0x67 0x00 0x6c 0x00 0x8600124: 0x6f 0x00 0x62 0x00 0x61 0x00 0x6c 0x00 0x860012c: 0x5d 0x00 0x00 0x00 0xa0 0x52 The string there reads "bject global]" + some junk because textlen is still 15. The pointer is off by n*sizeof(jschar) where n is the number of undefined objects before the "8" in the array. If you add more undefined elements before 8, then the pointer goes even further off. Maybe this helps to find the bug :)
Stupid mistake in my initial implementation.
Assignee: jorendorff → benjamin
Attachment #659757 - Flags: review?(jorendorff)
Attachment #659757 - Attachment description: decrease length appropiately → decrease length appropriately
Comment on attachment 659757 [details] [diff] [review] decrease length appropriately Review of attachment 659757 [details] [diff] [review]: ----------------------------------------------------------------- Thanks again, Benjamin! Please add two tests in string-contains.js (or wherever): assertEq("xyzzy".contains("zy\0", 2), false); var dots = Array(1000000).join('.'); assertEq(dots.contains("\x01", 1000000), false); assertEq(dots.contains("\0", 1000000), false); These should more reliably detect the bug (with a wrong answer or a crash, not just a valgrind warning). r=me with that.
Attachment #659757 - Flags: review?(jorendorff) → review+
Comment on attachment 659757 [details] [diff] [review] decrease length appropriately [Approval Request Comment] Bug caused by (feature/regressing bug #): Bug 772733 User impact if declined: JS reads memory which doesn't belong to it Testing completed (on m-c, etc.): Green try Risk to taking this patch (and alternatives if risky): None String or UUID changes made by this patch: None
Attachment #659757 - Flags: approval-mozilla-aurora?
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Attachment #659757 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Group: core-security
Whiteboard: [qa-]
A type of test for this bug has already been landed because it is already marked in-testsuite+ -> VERIFIED.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: