Preserving taint through arithmetic operations in Java
- 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 issue**
As described in issue #4845, by default CodeQL does not propagate taint across arithmetic operations (e.g. addition) for Java.
I'm working on a query where taint tracking across arithmetic operations is required and I plan on submitting a pull request for a query bug fix. I ran into an issue or two and could use some advice on the specifics.
**Background information**
I added an `isAdditionalFlowStep` predicate to the module implementing `DataFlow::ConfigSig`, then extended `AdditionalValueStep` with a subclass that uses the `ArithExpr` class from `semmle.code.java.arithmetic.Overflow`, like this:
```
import semmle.code.java.arithmetic.Overflow
class ArithmeticExpressionStep extends AdditionalValueStep {
override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
node2.asExpr().(ArithExpr).getAnOperand() = node1.asExpr()
}
}
module MyConfig implements DataFlow::ConfigSig {
// other predicates here
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
any(AdditionalValueStep r).step(node1, node2)
}
}
```
**Questions**
1. Although this fixes the issue for binary arithmetic operands, unary operands still don't propagate taint. `ArithExpr` already checks for `UnaryAssignExpr`. I also tried adding the check `node2.asExpr().(UnaryAssignExpr).getExpr() = node1.asExpr()` but it still doesn't work - I can't quite figure out the syntax for passing taint through a unary operator using the two `node1` and `node2` parameters. Any ideas?
2. Is there a better class for doing this than `ArithExpr` from `semmle.code.java.arithmetic.Overflow`? It seems like a reasonable choice, just a but strange such a low level class would be specific to overflow operations rather than generic. I wanted to verify the use was acceptable before doing a PR.
Thanks!
Guía de contribución
Línea de trabajo
Start with the DataFlow::ConfigSig module and its isAdditionalFlowStep predicate, then read AdditionalValueStep and semmle.code.java.arithmetic.Overflow, including ArithExpr and UnaryAssignExpr. Determine how unary and binary arithmetic nodes should propagate taint and whether ArithExpr is the appropriate abstraction; done means the behavior is understood well enough to support a focused query fix or pull request.
Escrito por el modelo de indexación a partir del texto del issue.
Evaluación
- Stack tecnológico
- java
- Área
- security
- Tipo de issue
- Error
- Dificultad
- 4/5
- Tiempo estimado
- 3-5 días
- Estado de actividad
- Estancado
- Claridad
- Bastante claro
- Aptitud para principiantes
- 35/100