[SERVER-68789] Coverity analysis defect 123004: Dereference before null check Created: 12/Aug/22  Updated: 27/Oct/23  Resolved: 06/Sep/22

Status: Closed
Project: Core Server
Component/s: Internal Code
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Minor - P4
Reporter: Coverity Collector User Assignee: Ivan Fefer
Resolution: Gone away Votes: 0
Labels: coverity, neweng
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: PNG File Screen Shot 2022-08-12 at 10.25.39 AM.png    
Sprint: QE 2022-09-19
Participants:

 Description   

Dereference before null check

There may be a null pointer dereference, or else the comparison against null is unnecessary. All paths that lead to this null pointer comparison already dereference the pointer earlier
/src/mongo/db/pipeline/sharded_agg_helpers.cpp:1006: REVERSE_INULL 123004 Directly dereferencing pointer "pipeline".
/src/mongo/db/pipeline/sharded_agg_helpers.cpp:1066: REVERSE_INULL 123004 Null-checking "pipeline" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.



 Comments   
Comment by Ivan Fefer [ 06/Sep/22 ]

I checked the code: pipeline null check was removed in this commit:

https://github.com/mongodb/mongo/commit/b99b5c3e242ff690b5127c5d672d226f7eaf5754#diff-cee99bda62dbaf162bfc87b6df57e6eb9d0816915d92e3b10646fbe91fb9fe52L1066

So now everything is consistent with assuming pipeline is not null. I checked some callers of this function: they all also assume pipeline is not null, so adding an assertion here seems random and useless.

Comment by Kyle Suarez [ 12/Aug/22 ]

Either pipeline should be checked for being null before the deference to get at the ExpressionContext, or the check for non-null pipeline on line 1066 should be elided since it's useless.

Generated at Thu Feb 08 06:11:44 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.