[CDRIVER-211] Use of uninitialized memory within the mongo_read_response function. Created: 26/Apr/13  Updated: 03/May/17  Resolved: 08/Aug/13

Status: Closed
Project: C Driver
Component/s: None
Affects Version/s: 0.7.1
Fix Version/s: None

Type: Bug Priority: Blocker - P1
Reporter: Tim Shelton Assignee: Gary Murakami
Resolution: Done Votes: 0
Labels: corrupt, crash
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

Use of uninitialized value on the stack (len). Fix is included. Hope to see this added

==4699== Thread 14:
==4699== Conditional jump or move depends on uninitialised value(s)
==4699== at 0x5DFE700: mongo_read_response (mongo.c:329)
==4699== by 0x5DFEE7E: mongo_cursor_op_query (mongo.c:1206)
==4699== by 0x5DFF3CC: mongo_cursor_next (mongo.c:1371)
==4699== by 0x5DFF4CC: mongo_find_one (mongo.c:1314)
==4699== by 0x5DFF5FA: mongo_run_command (mongo.c:1578)
==4699== by 0x5DFF85C: mongo_simple_int_command (mongo.c:1614)
==4699== by 0x5DFFB80: mongo_check_is_master (mongo.c:388)
==4699== by 0x5DFFCDF: mongo_client (mongo.c:438)
==4699== by 0x417E49: HawkMongoPersistentConnect (hawk-mongo-persistent.c:169)
==4699== by 0x4181B4: HawkMongoPopulateList (hawk-mongo-persistent.c:79)
==4699== by 0x408CE3: ModulesSensorReload (hawk-modules-reload.c:181)
==4699== by 0x8F6F850: start_thread (in /lib64/libpthread-2.12.so)
==4699== Uninitialised value was created by a stack allocation
==4699== at 0x5DFE6B0: mongo_read_response (mongo.c:317)
==4699==

Issue code:

static int mongo_read_response( mongo *conn, mongo_reply **reply ) {
mongo_header head; /* header from network */
mongo_reply_fields fields; /* header from network */
mongo_reply out; / native endian */
unsigned int len;
int res;

mongo_env_read_socket( conn, &head, sizeof( head ) );
mongo_env_read_socket( conn, &fields, sizeof( fields ) );

bson_little_endian32( &len, &head.len );

Fixed code:

static int mongo_read_response( mongo *conn, mongo_reply **reply ) {
mongo_header head; /* header from network */
mongo_reply_fields fields; /* header from network */
mongo_reply out; / native endian */
unsigned int len=0;
int res;

mongo_env_read_socket( conn, &head, sizeof( head ) );
mongo_env_read_socket( conn, &fields, sizeof( fields ) );

bson_little_endian32( &len, &head.len );

--------------------------------------------------------------------

— src/mongo.c 2013-04-26 14:59:47.791595035 -0500
+++ src/mongo.c.fix 2013-04-26 15:00:04.344604104 -0500
@@ -318,7 +318,7 @@
mongo_header head; /* header from network */
mongo_reply_fields fields; /* header from network */
mongo_reply out; / native endian */

  • unsigned int len;
    + unsigned int len=0;
    int res;

mongo_env_read_socket( conn, &head, sizeof( head ) );



 Comments   
Comment by Gary Murakami [ 08/Aug/13 ]

Agreed, fixed by CDRIVER-199

Comment by Paul Melnikow [ 04/Jul/13 ]

Looks like it was fixed in this commit:
https://github.com/mongodb/mongo-c-driver/commit/32875065e9743869742d7510263cdcd7de14aa61

Comment by Tim Shelton [ 04/Jul/13 ]

Yes, however if the read from the socket fails when writing to len value, bson_little_endian32 fails, then len is an undefined value and continues.

Valgrind doesn't lie. I'm just reporting what I find.

Tim

CTO
HAWK Network Defense, Inc.
1.888.919.4295 (HAWK) x 1337
214.810.4295 (HAWK)
tshelton@hawkdefense.com

Comment by Paul Melnikow [ 04/Jul/13 ]

This fix doesn't make sense. The call to bson_little_endian32 writes to len; it doesn't depend on the value of len.

Generated at Wed Feb 07 21:08:47 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.