[SERVER-7010] ObjectId(ObjectId("4e859938e4b0c81c1411b0c7")) triggers stackdump Created: 11/Sep/12  Updated: 11/Jul/16  Resolved: 11/Sep/12

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: 2.2.0
Fix Version/s: 2.3.0

Type: Bug Priority: Major - P3
Reporter: Jean-Marie Gaillourdet Assignee: Tad Marshall
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified
Environment:

OS X as well as Linux using the mongo shell


Issue Links:
Duplicate
is duplicated by SERVER-7590 Mongo shell assertion violation when ... Closed
Backwards Compatibility: Fully Compatible
Operating System: ALL
Participants:

 Description   

tomcat@luna334:~$ mongo --port 27019
MongoDB shell version: 2.2.0
connecting to: 127.0.0.1:27019/test
REPLICA1:PRIMARY> ObjectId(ObjectId("4e859938e4b0c81c1411b0c7"))
Tue Sep 11 13:57:09 Assertion: 10448:invalid object id: length
0x511fa1 0x573f0b 0x5648e1 0x70ce24 0x6bdf6c 0x6af9e5 0x6bdaff 0x584c26 0x584ca2 0x5860ae 0x71e6d4 0x4a43e6 0x4a6726 0x7f2f8298cc4d 0x49f5a9
mongo(_ZN5mongo15printStackTraceERSo+0x21) [0x511fa1]
mongo(_ZN5mongo11msgassertedEiPKc+0x9b) [0x573f0b]
mongo() [0x5648e1]
mongo(ZN5mongo21object_id_constructorEP9JSContextP8JSObjectjPlS4+0x2b4) [0x70ce24]
mongo(js_Invoke+0x40c) [0x6bdf6c]
mongo(js_Interpret+0x1305) [0x6af9e5]
mongo(js_Execute+0x36f) [0x6bdaff]
mongo(JS_EvaluateUCScriptForPrincipals+0x66) [0x584c26]
mongo(JS_EvaluateUCScript+0x22) [0x584ca2]
mongo(JS_EvaluateScript+0x6e) [0x5860ae]
mongo(_ZN5mongo7SMScope4execERKNS_10StringDataERKSsbbbi+0x144) [0x71e6d4]
mongo(_Z5_mainiPPc+0x2156) [0x4a43e6]
mongo(main+0x26) [0x4a6726]
/lib/libc.so.6(__libc_start_main+0xfd) [0x7f2f8298cc4d]
mongo(__gxx_personality_v0+0x2a1) [0x49f5a9]
Tue Sep 11 13:57:09 Error: invalid object id: length (shell):1
REPLICA1:PRIMARY>

While the expression might be nonsensical it should definitely lead to a proper runtime error not to a stacktrace.



 Comments   
Comment by auto [ 11/Sep/12 ]

Author:

{u'date': u'2012-09-11T07:16:13-07:00', u'email': u'tad@10gen.com', u'name': u'Tad Marshall'}

Message: SERVER-7010 do not use massert to validate user input

Change two uses of massert() to uassert() in
Scope::validateObjectIdString() to prevent stack trace on
an invalid string supplied to the ObjectId() constructor.
Branch: master
https://github.com/mongodb/mongo/commit/d780f7aa2bda5bb767e09b3632a35cd00ba45a95

Comment by Tad Marshall [ 11/Sep/12 ]

I checked engine.cpp and engine_spidermonkey.cpp for other masserts and the only additional ones I found were for actual failures, not validation, so they are correct.

Comment by Tad Marshall [ 11/Sep/12 ]

It looks like anything unexpected passed as an argument to ObjectId() generates a stack trace.

MongoDB shell version: 2.3.0-pre-
> ObjectId()
ObjectId("504f29bf391883d77ddad9a6")
> ObjectId(0)
Tue Sep 11 08:08:36 Assertion: 10448:invalid object id: length
0x5b74a3 0x587cd4 0x5693bd 0x57545a 0x657f8c 0x64bbdc 0x657979 0x619dba 0x619e42 0x619ebe 0x584cc9 0x4b6e7d 0x4ae04e 0x7f3108bd776d 0x4b2769 
 ./mongo(_ZN5mongo15printStackTraceERSo+0x23) [0x5b74a3]
 ./mongo(_ZN5mongo11msgassertedEiPKc+0x94) [0x587cd4]
 ./mongo() [0x5693bd]
 ./mongo(_ZN5mongo21object_id_constructorEP9JSContextP8JSObjectjPlS4_+0x16a) [0x57545a]
 ./mongo(js_Invoke+0x4bc) [0x657f8c]
 ./mongo(js_Interpret+0x121c) [0x64bbdc]
 ./mongo(js_Execute+0x269) [0x657979]
 ./mongo(JS_EvaluateUCScriptForPrincipals+0x6a) [0x619dba]
 ./mongo(JS_EvaluateUCScript+0x22) [0x619e42]
 ./mongo(JS_EvaluateScript+0x6e) [0x619ebe]
 ./mongo(_ZN5mongo7SMScope4execERKNS_10StringDataERKSsbbbi+0x139) [0x584cc9]
 ./mongo(_Z5_mainiPPc+0x1e6d) [0x4b6e7d]
 ./mongo(main+0x1e) [0x4ae04e]
 /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xed) [0x7f3108bd776d]
 ./mongo() [0x4b2769]
Tue Sep 11 08:08:36 Error: invalid object id: length (shell):1
> 

Version 2.0.7 does the same thing.

In src/mongo/scripting/engine.cpp at line 161 (in today's master code), the Scope::validateObjectIdString() routine uses an massert test that the string length is exactly 24, and then does another massert on line 170 if a non-hex character is found.

Because this is our first test of user-supplied values, these should be uassert's (User Assertions) and not massert's (Message Assertions). This would prevent the stack trace.

We might want to audit this code to see if other validation tests of user input use massert when uassert would be a better choice.

Generated at Thu Feb 08 03:13:23 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.