[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)
/src/mongo/db/pipeline/expression.cpp, line: 77
|
| Comments |
| Comment by Githook User [ 26/Mar/19 ] | ||
|
Author: {'email': 'arun.banala@mongodb.com', 'name': 'Arun Banala', 'username': 'banarun'}Message: | ||
| 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.
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. |