Files
codeql/javascript/ql/src/LanguageFeatures/LengthComparisonOffByOne.ql
2019-01-07 10:15:45 +00:00

67 lines
2.2 KiB
Plaintext

/**
* @name Off-by-one comparison against length
* @description An array index is compared to be less than or equal to the 'length' property,
* and then used in an indexing operation that could be out of bounds.
* @kind problem
* @problem.severity warning
* @id js/index-out-of-bounds
* @tags reliability
* correctness
* logic
* external/cwe/cwe-193
* @precision high
*/
import javascript
/**
* Gets an access to `array.length`.
*/
PropAccess arrayLen(Variable array) { result.accesses(array.getAnAccess(), "length") }
/**
* Gets a condition that checks that `index` is less than or equal to `array.length`.
*/
ConditionGuardNode getLengthLEGuard(Variable index, Variable array) {
exists(RelationalComparison cmp | cmp instanceof GEExpr or cmp instanceof LEExpr |
cmp = result.getTest() and
result.getOutcome() = true and
cmp.getGreaterOperand() = arrayLen(array) and
cmp.getLesserOperand() = index.getAnAccess()
)
}
/**
* Gets a condition that checks that `index` is not equal to `array.length`.
*/
ConditionGuardNode getLengthNEGuard(Variable index, Variable array) {
exists(EqualityTest eq |
eq = result.getTest() and
result.getOutcome() = eq.getPolarity().booleanNot() and
eq.hasOperands(index.getAnAccess(), arrayLen(array))
)
}
/**
* Holds if `ea` is a read from `array[index]` in basic block `bb`.
*/
predicate elementRead(IndexExpr ea, Variable array, Variable index, BasicBlock bb) {
ea.getBase() = array.getAnAccess() and
ea.getIndex() = index.getAnAccess() and
ea instanceof RValue and
bb = ea.getBasicBlock()
}
from ConditionGuardNode cond, Variable array, Variable index, IndexExpr ea, BasicBlock bb
where
// there is a comparison `index <= array.length`
cond = getLengthLEGuard(index, array) and
// there is a read from `array[index]`
elementRead(ea, array, index, bb) and
// and the read is guarded by the comparison
cond.dominates(bb) and
// but the read is not guarded by another check that `index != array.length`
not getLengthNEGuard(index, array).dominates(bb)
select cond.getTest(), "Off-by-one index comparison against length may lead to out-of-bounds $@.",
ea, "read"