github / github/codeql

False positive: cpp/path-injection

Aperta
#12,924 0 commenti 0 reazioni 1 assegnatario Rivendicata da @MathiasVP Vedi su GitHub
C++ false-positive
Lingua principale
CodeQL
Stelle
10.1k
Fork
2.1k
Merge medio
2g 15h
PR unite (30g)
141

Descrizione

**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");
}
```

Guida per i contributori

Apri la guida per i contributori

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.