github / github/codeql

False positive: cpp/path-injection

Ouverte
#12,924 0 commentaires 0 réactions 1 personne assignée Réclamée par @MathiasVP Voir sur GitHub
C++ false-positive
Langage dominant
CodeQL
Étoiles
10.1k
Forks
2.1k
Merge moyen
2 j 15 h
PR mergées (30 j)
141

Description

**Description of the false positive**

`cpp/path-injection` fails to detect validations not involving 'upper bound checks', as is currently used in the isBarrier predicate.

While a validation routine is generally difficult to detect or to know if it is complete, I believe the current query attempts to approximate it via two methods:
1) if data flows out of a function that converts the user input into an integer, or
2) an operation where an 'upper bound check', i.e., a relational operator is used.

This is the current data flow barrier predicate:
```
predicate isBarrier(DataFlow::Node node) {
node.asExpr().(Call).getTarget().getUnspecifiedType() instanceof ArithmeticType
or
exists(LoadInstruction load, Variable checkedVar |
load = node.asInstruction() and
checkedVar = load.getSourceAddress().(VariableAddressInstruction).getAstVariable() and
hasUpperBoundsCheck(checkedVar)
)
}
```

The use of a `LoadInstruction` also appears a bit odd, and I'm not sure if that is necessary for the kind of problem being solved.

`hasUpperBoundsCheck` is defined as follows:

```
predicate hasUpperBoundsCheck(Variable var) {
exists(RelationalOperation oper, VariableAccess access |
oper.getAnOperand() = access and
access.getTarget() = var and
// Comparing to 0 is not an upper bound check
not oper.getAnOperand().getValue() = "0"
)
}
```

This appears to only consider validation to be valid for relational operators, but does not consider equality operators, which is a common use case.

**Code samples or links to source code**
Below is an example program and validation function that relies on an inequality operation. A path from argv to the open is still detected with the current query resulting in false postives.

```
#include
#include
#include

int validateName(char* name){
if (strchr(name, '/') != NULL){
return -1;
}
}

int main(int argc, char**argv){
char* fileName = argv[0];
FILE *fd;
if(validateName(fileName) == -1){
exit(1);
}
fd = fopen(fileName, "r");
}
```

Guide de contribution

Ouvrir le guide de contribution

Évaluation

Cette issue n'a pas encore été évaluée.

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.