github / github/codeql

Preserving taint through arithmetic operations in Java

Abierto
#14,233 4 comentarios 0 reacciones 0 asignados Ver en GitHub
question
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

Abrir la 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

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.