[SERVER-40140] Coverity analysis defect 112121: Improper use of negative value Created: 14/Mar/19  Updated: 29/Oct/23  Resolved: 26/Mar/19

Status: Closed
Project: Core Server
Component/s: Aggregation Framework
Affects Version/s: None
Fix Version/s: 4.1.9

Type: Bug Priority: Major - P3
Reporter: Coverity Collector User Assignee: Arun Banala
Resolution: Fixed Votes: 0
Labels: coverity
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Backwards Compatibility: Fully Compatible
Operating System: ALL
Sprint: Query 2019-03-25, Query 2019-04-08
Participants:

 Description   

Negative value can be returned from function is not being checked before being used improperly

Defect 112121 (STATIC_C)
Checker NEGATIVE_RETURNS (subcategory none)
File: /src/mongo/db/pipeline/expression.cpp
Function mongo::Expression::removeFieldPrefix(const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> &)::[lambda() (instance 1)]::operator ()() const
/src/mongo/db/pipeline/expression.cpp, line: 77
Function "this->prefixedField->find("", 0UL)" returns a negative number. [Note: The source code implementation of the function has been overridden by a builtin model.]

        uassert(16419,

/src/mongo/db/pipeline/expression.cpp, line: 77
Assigning: "<temporary>" = "this->prefixedField->find("", 0UL)".

        uassert(16419,



 Comments   
Comment by Githook User [ 26/Mar/19 ]

Author:

{'email': 'arun.banala@mongodb.com', 'name': 'Arun Banala', 'username': 'banarun'}

Message: SERVER-40140 Fix Coverity analysis defect: Improper use of negative value
Branch: master
https://github.com/mongodb/mongo/commit/233a4f7cfdf13fa0cfa55c6209644f7e3dbbed4b

Comment by Arun Banala [ 20/Mar/19 ]

The failure occurs here. The issue is, uassert has a condition on prefixedField.find('\0') where as in the error message prefixedField.find("\0") is used. Since it becomes a different find condition, it could potentially return a negative number. The use of prefixedField.find("\0") instead of prefixedField.find('\0') looks like a bug. The find("\0") will match any string and always return zero, no matter what the prefixedField is. Where as find('\0') will return the index of first null character and string::Pos if there is no null character.

There is another issue with the way error message is formatted. I was able to produce the error below. I think the author's intention was to print the position at which the the null character exists.

> db.coll.aggregate([ {$unwind : "$ab\0c"}])> db.coll.aggregate([ {$unwind : "$ab\0c"}])2019-03-20T11:11:52.836+0000 E QUERY    [js] uncaught exception: Error: command failed: { "ok" : 0, "errmsg" : "field path must not contain embedded null characters0,", "code" : 16419, "codeName" : "Location16419"} : aggregate failed :

The simplest solution is to remove prefixedField.find("\0") statement from error message, as the position of the null character is not very helpful. 

Comment by Eric Milkie [ 14/Mar/19 ]

I'm not sure what that uassert string was supposed to be, but I suspect today it just appends a number between the message and that random comma at the end.

Generated at Thu Feb 08 04:54:08 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.