mirror of
https://github.com/github/codeql.git
synced 2025-12-18 09:43:15 +01:00
add backtracking to find division that end up being rounded
This commit is contained in:
@@ -132,6 +132,18 @@ DataFlow::Node goodRandom(DataFlow::SourceNode source) {
|
|||||||
result = goodRandom(DataFlow::TypeTracker::end(), source)
|
result = goodRandom(DataFlow::TypeTracker::end(), source)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Gets a node that is passed to a rounding function from `Math`, using type-backtracker `t`.
|
||||||
|
*/
|
||||||
|
DataFlow::Node isRounded(DataFlow::TypeBackTracker t) {
|
||||||
|
t.start() and
|
||||||
|
result = DataFlow::globalVarRef("Math").getAMemberCall(["round", "floor", "ceil"]).getArgument(0)
|
||||||
|
or
|
||||||
|
exists(DataFlow::TypeBackTracker t2 | t2 = t.smallstep(result, isRounded(t2)))
|
||||||
|
or
|
||||||
|
InsecureRandomness::isAdditionalTaintStep(result, isRounded(t.continue()))
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Gets a node that that produces a biased result from otherwise cryptographically secure random numbers produced by `source`.
|
* Gets a node that that produces a biased result from otherwise cryptographically secure random numbers produced by `source`.
|
||||||
*/
|
*/
|
||||||
@@ -153,10 +165,7 @@ DataFlow::Node badCrypto(string description, DataFlow::SourceNode source) {
|
|||||||
goodRandom(source).asExpr() = div.getLeftOperand() and
|
goodRandom(source).asExpr() = div.getLeftOperand() and
|
||||||
description = "division and rounding the result" and
|
description = "division and rounding the result" and
|
||||||
not div.getRightOperand() = isPowerOfTwoMinusOne().asExpr() and // division by (2^n)-1 most of the time produces a uniformly random number between 0 and 1.
|
not div.getRightOperand() = isPowerOfTwoMinusOne().asExpr() and // division by (2^n)-1 most of the time produces a uniformly random number between 0 and 1.
|
||||||
DataFlow::globalVarRef("Math")
|
div = isRounded(DataFlow::TypeBackTracker::end()).asExpr()
|
||||||
.getAMemberCall(["round", "floor", "ceil"])
|
|
||||||
.getArgument(0)
|
|
||||||
.asExpr() = div
|
|
||||||
)
|
)
|
||||||
or
|
or
|
||||||
// modulo - only bad if not by a power of 2 - and the result is not checked for bias
|
// modulo - only bad if not by a power of 2 - and the result is not checked for bias
|
||||||
|
|||||||
@@ -13,3 +13,5 @@
|
|||||||
| bad-random.js:85:11:85:35 | goodRan ... Random2 | Using addition on a $@ produces biased results. | bad-random.js:84:23:84:38 | secureRandom(10) | cryptographically secure random number |
|
| bad-random.js:85:11:85:35 | goodRan ... Random2 | Using addition on a $@ produces biased results. | bad-random.js:84:23:84:38 | secureRandom(10) | cryptographically secure random number |
|
||||||
| bad-random.js:87:16:87:24 | bad + bad | Using addition on a $@ produces biased results. | bad-random.js:83:23:83:38 | secureRandom(10) | cryptographically secure random number |
|
| bad-random.js:87:16:87:24 | bad + bad | Using addition on a $@ produces biased results. | bad-random.js:83:23:83:38 | secureRandom(10) | cryptographically secure random number |
|
||||||
| bad-random.js:87:16:87:24 | bad + bad | Using addition on a $@ produces biased results. | bad-random.js:84:23:84:38 | secureRandom(10) | cryptographically secure random number |
|
| bad-random.js:87:16:87:24 | bad + bad | Using addition on a $@ produces biased results. | bad-random.js:84:23:84:38 | secureRandom(10) | cryptographically secure random number |
|
||||||
|
| bad-random.js:90:29:90:54 | secureR ... / 25.6 | Using division and rounding the result on a $@ produces biased results. | bad-random.js:90:29:90:44 | secureRandom(10) | cryptographically secure random number |
|
||||||
|
| bad-random.js:96:29:96:58 | crypto. ... ] / 100 | Using division and rounding the result on a $@ produces biased results. | bad-random.js:96:29:96:49 | crypto. ... ytes(1) | cryptographically secure random number |
|
||||||
|
|||||||
@@ -87,13 +87,13 @@ var bad = goodRandom1 + goodRandom2; // NOT OK
|
|||||||
var dontFlag = bad + bad; // OK - the operands have already been flagged - but flagged anyway due to us not detecting that [INCONSISTENCY].
|
var dontFlag = bad + bad; // OK - the operands have already been flagged - but flagged anyway due to us not detecting that [INCONSISTENCY].
|
||||||
|
|
||||||
var good = secureRandom(10)[0] / 0xff; // OK - result is not rounded.
|
var good = secureRandom(10)[0] / 0xff; // OK - result is not rounded.
|
||||||
var good = Math.ceil(0.5 - (secureRandom(10)[0] / 25.6)); // NOT OK - division generally introduces bias - but not flagged due to not looking through nested arithmetic [INCONSISTENCY].
|
var good = Math.ceil(0.5 - (secureRandom(10)[0] / 25.6)); // NOT OK - division generally introduces bias - but not flagged due to not looking through nested arithmetic.
|
||||||
|
|
||||||
var good = (crypto.randomBytes(1)[0] << 8) + crypto.randomBytes(3)[0]; // OK - bit shifts are usually used to construct larger/smaller numbers,
|
var good = (crypto.randomBytes(1)[0] << 8) + crypto.randomBytes(3)[0]; // OK - bit shifts are usually used to construct larger/smaller numbers,
|
||||||
|
|
||||||
var good = Math.floor(max * (crypto.randomBytes(1)[0] / 0xff)); // OK - division by 0xff (255) gives a uniformly random number between 0 and 1.
|
var good = Math.floor(max * (crypto.randomBytes(1)[0] / 0xff)); // OK - division by 0xff (255) gives a uniformly random number between 0 and 1.
|
||||||
|
|
||||||
var bad = Math.floor(max * (crypto.randomBytes(1)[0] / 100)); // NOT OK - division by 100 gives bias - but not flagged due to not looking through nested arithmetic [INCONSISTENCY].
|
var bad = Math.floor(max * (crypto.randomBytes(1)[0] / 100)); // NOT OK - division by 100 gives bias - but not flagged due to not looking through nested arithmetic.
|
||||||
|
|
||||||
var crb = crypto.randomBytes(4);
|
var crb = crypto.randomBytes(4);
|
||||||
var cryptoRand = 0x01000000 * crb[0] + 0x00010000 * crb[1] + 0x00000100 * crb[2] + 0x00000001 * crb[3]; // OK - producing a larger number from smaller numbers.
|
var cryptoRand = 0x01000000 * crb[0] + 0x00010000 * crb[1] + 0x00000100 * crb[2] + 0x00000001 * crb[3]; // OK - producing a larger number from smaller numbers.
|
||||||
|
|||||||
Reference in New Issue
Block a user