github / github/codeql

False positive: cpp/path-injection

Abierto
#12,924 0 comentarios 0 reacciones 1 asignado Reclamado por @MathiasVP Ver en GitHub
C++ false-positive
Lenguaje dominante
CodeQL
Estrellas
10.1k
Forks
2.1k
Merge medio
2 d 15 h
PR fusionados (30 d)
141

Descripción

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

Guía de contribución

Abrir la guía de contribución

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.