[CDRIVER-3513] Fix instances where functions may return false without setting the out bson_error_t Created: 31/Jan/20  Updated: 28/Oct/23  Resolved: 01/Jul/20

Status: Closed
Project: C Driver
Component/s: libmongoc
Affects Version/s: None
Fix Version/s: 1.17.0-rc0, 1.17.0

Type: Bug Priority: Major - P3
Reporter: Kevin Albertson Assignee: Andrew Witten (Inactive)
Resolution: Fixed Votes: 0
Labels: codeql, new-eng
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
is related to CDRIVER-3332 Kerberos auth with Windows SSPI broke... Closed

 Description   

A common pattern for functions in libmongoc is to return a boolean and have an out parameter bson_error_t. If the return value is false, callers can assume the bson_error_t was set. In the past, we've discovered bugs due to code not abiding by this contract (e.g. CDRIVER-3332).

The following cases appear to be times where functions return false without setting the out bson_error_t param when they should be:

Instances were found using this CodeQL query and skimming through the 54 results:

import cpp
 
from Function f, Parameter err, ReturnStmt r
where
// Check that f has a bson_error_t * output parameter.
err.getType().getName() = "bson_error_t *" and
err.getFunction() = f and
// And that f returns a boolean
f.getType().getName() = "bool" and
// And there is a return false;
r.getExpr().(Literal).getValue().toInt() = 0 and
r.getEnclosingFunction() = f and
// no a call to bson_set_error in the same block.
not exists (FunctionCall fc |
    fc.getTarget().getQualifiedName() = "bson_set_error" and
    r.getEnclosingBlock() = fc.getEnclosingBlock()
) and
// no a call to a function taking a bson_error_t in the same block.
not exists (FunctionCall fc, Parameter arg |
    arg.getName() = err.getName() and
    arg.getFunction() = fc.getTarget() and
    r.getEnclosingBlock() = fc.getEnclosingBlock()
)
and
// no call to a function in an if condition containing
not exists (FunctionCall fc, Parameter arg, IfStmt ifstmt |
    arg.getName() = err.getName() and
    arg.getFunction() = fc.getTarget() and
    fc.getParent*() = ifstmt.getCondition() and
    r.getParent*() = ifstmt.getThen()
)
// Not a call to bson_set_error in the if condition of the nested block
select r, f

Can be run here (or locally): https://lgtm.com/query/8792356789210846853/



 Comments   
Comment by Githook User [ 11/Jul/20 ]

Author:

{'name': 'Andrew Witten', 'email': 'andrew.witten@mongodb.com', 'username': 'awitten1'}

Message: CDRIVER-3513 saved errors in out parameter (#650)

saved errors and added test case
Branch: r1.17
https://github.com/mongodb/mongo-c-driver/commit/dacfd414dd995e2c11f5290e19b9c6a656af7aeb

Comment by Githook User [ 01/Jul/20 ]

Author:

{'name': 'Andrew Witten', 'email': 'andrew.witten@mongodb.com', 'username': 'awitten1'}

Message: CDRIVER-3513 saved errors in out parameter (#650)

saved errors and added test case
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/368439f8410c39234f9d82d8466202176bf9d57a

Comment by Andrew Witten (Inactive) [ 29/Jun/20 ]

PR: https://github.com/mongodb/mongo-c-driver/pull/650

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