Skip to content

Commit 43d8e83

Browse files
geoffw0geoffw0
committed
Merge branch 'main' into taintbits
2 parents 6c40e22 + 5d1c2a3 commit 43d8e83

File tree

24 files changed

+609
-70
lines changed

24 files changed

+609
-70
lines changed

‎.devcontainer/devcontainer.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,6 @@
44
"slevesque.vscode-zipexplorer"
55
],
66
"settings": {
7-
"codeQL.experimentalBqrsParsing": true
7+
"codeQL.runningQueries.memory": 2048
88
}
99
}

‎.github/workflows/labeler.yml‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
name: "Pull Request Labeler"
2+
on:
3+
- pull_request_target
4+
5+
jobs:
6+
triage:
7+
runs-on: ubuntu-latest
8+
steps:
9+
- uses: actions/labeler@v2
10+
with:
11+
repo-token: "${{ secrets.GITHUB_TOKEN }}"

‎cpp/ql/src/semmle/code/cpp/rangeanalysis/SimpleRangeAnalysis.qll‎

Lines changed: 62 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1144,6 +1144,17 @@ private float getPhiLowerBounds(StackVariable v, RangeSsaDefinition phi) {
11441144
if guardLB > defLB then result = guardLB else result = defLB
11451145
)
11461146
or
1147+
exists(VariableAccess access, float neConstant, float lower |
1148+
isNEPhi(v, phi, access, neConstant) and
1149+
lower = getFullyConvertedLowerBounds(access) and
1150+
if lower = neConstant then result = lower + 1 else result = lower
1151+
)
1152+
or
1153+
exists(VariableAccess access |
1154+
isUnsupportedGuardPhi(v, phi, access) and
1155+
result = getFullyConvertedLowerBounds(access)
1156+
)
1157+
or
11471158
result = getDefLowerBounds(phi.getAPhiInput(v), v)
11481159
}
11491160

@@ -1161,6 +1172,17 @@ private float getPhiUpperBounds(StackVariable v, RangeSsaDefinition phi) {
11611172
if guardUB < defUB then result = guardUB else result = defUB
11621173
)
11631174
or
1175+
exists(VariableAccess access, float neConstant, float upper |
1176+
isNEPhi(v, phi, access, neConstant) and
1177+
upper = getFullyConvertedUpperBounds(access) and
1178+
if upper = neConstant then result = upper - 1 else result = upper
1179+
)
1180+
or
1181+
exists(VariableAccess access |
1182+
isUnsupportedGuardPhi(v, phi, access) and
1183+
result = getFullyConvertedUpperBounds(access)
1184+
)
1185+
or
11641186
result = getDefUpperBounds(phi.getAPhiInput(v), v)
11651187
}
11661188

@@ -1423,22 +1445,13 @@ private predicate linearBoundFromGuard(
14231445
// 1. x <= upperbound(RHS)
14241446
// 2. x >= lowerbound(RHS)
14251447
//
1426-
// For x != RHS, we create trivial bounds:
1427-
//
1428-
// 1. x <= typeUpperBound(RHS.getUnspecifiedType())
1429-
// 2. x >= typeLowerBound(RHS.getUnspecifiedType())
1430-
//
1431-
exists(Expr lhs, Expr rhs, boolean isEQ |
1448+
exists(Expr lhs, Expr rhs |
14321449
linearAccess(lhs, v, p, q) and
1433-
eqOpWithSwapAndNegate(guard, lhs, rhs, isEQ, branch) and
1450+
eqOpWithSwapAndNegate(guard, lhs, rhs, true, branch) and
1451+
getBounds(rhs, boundValue, isLowerBound) and
14341452
strictness = Nonstrict()
1435-
|
1436-
// True branch
1437-
isEQ = true and getBounds(rhs, boundValue, isLowerBound)
1438-
or
1439-
// False branch: set the bounds to the min/max for the type.
1440-
isEQ = false and exprTypeBounds(rhs, boundValue, isLowerBound)
14411453
)
1454+
// x != RHS and !x are handled elsewhere
14421455
}
14431456

14441457
/** Utility for `linearBoundFromGuard`. */
@@ -1455,6 +1468,42 @@ private predicate exprTypeBounds(Expr expr, float boundValue, boolean isLowerBou
14551468
isLowerBound = false and boundValue = exprMaxVal(expr.getFullyConverted())
14561469
}
14571470

1471+
/**
1472+
* Holds if `(v, phi)` ensures that `access` is not equal to `neConstant`. For
1473+
* example, the condition `if (x + 1 != 3)` ensures that `x` is not equal to 2.
1474+
* Only integral types are supported.
1475+
*/
1476+
private predicate isNEPhi(
1477+
Variable v, RangeSsaDefinition phi, VariableAccess access, float neConstant
1478+
) {
1479+
exists(
1480+
ComparisonOperation cmp, boolean branch, Expr linearExpr, Expr rExpr, float p, float q, float r
1481+
|
1482+
access.getTarget() = v and
1483+
phi.isGuardPhi(access, cmp, branch) and
1484+
eqOpWithSwapAndNegate(cmp, linearExpr, rExpr, false, branch) and
1485+
v.getUnspecifiedType() instanceof IntegralOrEnumType and // Float `!=` is too imprecise
1486+
r = getValue(rExpr).toFloat() and
1487+
linearAccess(linearExpr, access, p, q) and
1488+
neConstant = (r - q) / p
1489+
)
1490+
}
1491+
1492+
/**
1493+
* Holds if `(v, phi)` constrains the value of `access` but in a way that
1494+
* doesn't allow this library to constrain the upper or lower bounds of
1495+
* `access`. An example is `if (x != y)` if neither `x` nor `y` is a
1496+
* compile-time constant.
1497+
*/
1498+
private predicate isUnsupportedGuardPhi(Variable v, RangeSsaDefinition phi, VariableAccess access) {
1499+
exists(ComparisonOperation cmp, boolean branch |
1500+
access.getTarget() = v and
1501+
phi.isGuardPhi(access, cmp, branch) and
1502+
eqOpWithSwapAndNegate(cmp, _, _, false, branch) and
1503+
not isNEPhi(v, phi, access, _)
1504+
)
1505+
}
1506+
14581507
cached
14591508
private module SimpleRangeAnalysisCached {
14601509
/**

‎cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/lowerBound.expected‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,37 @@
532532
| test.c:530:3:530:3 | i | -2147483648 |
533533
| test.c:530:10:530:11 | sc | 1 |
534534
| test.c:532:7:532:7 | i | -128 |
535+
| test.c:539:7:539:7 | n | 0 |
536+
| test.c:541:7:541:7 | n | 0 |
537+
| test.c:542:9:542:9 | n | 1 |
538+
| test.c:545:7:545:7 | n | 0 |
539+
| test.c:546:9:546:9 | n | 1 |
540+
| test.c:548:9:548:9 | n | 0 |
541+
| test.c:551:8:551:8 | n | 0 |
542+
| test.c:552:9:552:9 | n | 0 |
543+
| test.c:554:9:554:9 | n | 0 |
544+
| test.c:557:10:557:10 | n | 0 |
545+
| test.c:558:5:558:5 | n | 1 |
546+
| test.c:561:7:561:7 | n | 0 |
547+
| test.c:565:7:565:7 | n | -32768 |
548+
| test.c:568:7:568:7 | n | 0 |
549+
| test.c:569:9:569:9 | n | 0 |
550+
| test.c:571:9:571:9 | n | 1 |
551+
| test.c:574:7:574:7 | n | 0 |
552+
| test.c:575:9:575:9 | n | 0 |
553+
| test.c:577:9:577:9 | n | 0 |
554+
| test.c:580:10:580:10 | n | 0 |
555+
| test.c:581:5:581:5 | n | 1 |
556+
| test.c:584:7:584:7 | n | 0 |
557+
| test.c:588:7:588:7 | n | -32768 |
558+
| test.c:589:9:589:9 | n | -32768 |
559+
| test.c:590:11:590:11 | n | 0 |
560+
| test.c:594:7:594:7 | n | -32768 |
561+
| test.c:595:13:595:13 | n | 5 |
562+
| test.c:598:9:598:9 | n | 6 |
563+
| test.c:601:7:601:7 | n | -32768 |
564+
| test.c:601:22:601:22 | n | -32767 |
565+
| test.c:602:9:602:9 | n | -32766 |
535566
| test.cpp:10:7:10:7 | b | -2147483648 |
536567
| test.cpp:11:5:11:5 | x | -2147483648 |
537568
| test.cpp:13:10:13:10 | x | -2147483648 |

‎cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/test.c‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -533,3 +533,72 @@ int mul_by_constant(int i, int j) {
533533

534534
return 0;
535535
}
536+
537+
538+
int notequal_type_endpoint(unsigned n) {
539+
out(n); // 0 ..
540+
541+
if (n > 0) {
542+
out(n); // 1 ..
543+
}
544+
545+
if (n != 0) {
546+
out(n); // 1 ..
547+
} else {
548+
out(n); // 0 .. 0
549+
}
550+
551+
if (!n) {
552+
out(n); // 0 .. 0
553+
} else {
554+
out(n); // 1 .. [BUG: lower bound is deduced to be 0]
555+
}
556+
557+
while (n != 0) {
558+
n--; // 1 ..
559+
}
560+
561+
out(n); // 0 .. 0
562+
}
563+
564+
void notequal_refinement(short n) {
565+
if (n < 0)
566+
return;
567+
568+
if (n == 0) {
569+
out(n); // 0 .. 0
570+
} else {
571+
out(n); // 1 ..
572+
}
573+
574+
if (n) {
575+
out(n); // 1 .. [BUG: lower bound is deduced to be 0]
576+
} else {
577+
out(n); // 0 .. 0
578+
}
579+
580+
while (n != 0) {
581+
n--; // 1 ..
582+
}
583+
584+
out(n); // 0 .. 0
585+
}
586+
587+
void notequal_variations(short n, float f) {
588+
if (n != 0) {
589+
if (n >= 0) {
590+
out(n); // 1 .. [BUG: we can't handle `!=` coming first]
591+
}
592+
}
593+
594+
if (n >= 5) {
595+
if (2 * n - 10 == 0) { // Same as `n == 10/2` (modulo overflow)
596+
return;
597+
}
598+
out(n); // 6 ..
599+
}
600+
601+
if (n != -32768 && n != -32767) {
602+
out(n); // -32766 ..
603+
}
604+
}

‎cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/upperBound.expected‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,37 @@
532532
| test.c:530:3:530:3 | i | 2147483647 |
533533
| test.c:530:10:530:11 | sc | 1 |
534534
| test.c:532:7:532:7 | i | 127 |
535+
| test.c:539:7:539:7 | n | 4294967295 |
536+
| test.c:541:7:541:7 | n | 4294967295 |
537+
| test.c:542:9:542:9 | n | 4294967295 |
538+
| test.c:545:7:545:7 | n | 4294967295 |
539+
| test.c:546:9:546:9 | n | 4294967295 |
540+
| test.c:548:9:548:9 | n | 0 |
541+
| test.c:551:8:551:8 | n | 4294967295 |
542+
| test.c:552:9:552:9 | n | 4294967295 |
543+
| test.c:554:9:554:9 | n | 4294967295 |
544+
| test.c:557:10:557:10 | n | 4294967295 |
545+
| test.c:558:5:558:5 | n | 4294967295 |
546+
| test.c:561:7:561:7 | n | 0 |
547+
| test.c:565:7:565:7 | n | 32767 |
548+
| test.c:568:7:568:7 | n | 32767 |
549+
| test.c:569:9:569:9 | n | 0 |
550+
| test.c:571:9:571:9 | n | 32767 |
551+
| test.c:574:7:574:7 | n | 32767 |
552+
| test.c:575:9:575:9 | n | 32767 |
553+
| test.c:577:9:577:9 | n | 32767 |
554+
| test.c:580:10:580:10 | n | 32767 |
555+
| test.c:581:5:581:5 | n | 32767 |
556+
| test.c:584:7:584:7 | n | 0 |
557+
| test.c:588:7:588:7 | n | 32767 |
558+
| test.c:589:9:589:9 | n | 32767 |
559+
| test.c:590:11:590:11 | n | 32767 |
560+
| test.c:594:7:594:7 | n | 32767 |
561+
| test.c:595:13:595:13 | n | 32767 |
562+
| test.c:598:9:598:9 | n | 32767 |
563+
| test.c:601:7:601:7 | n | 32767 |
564+
| test.c:601:22:601:22 | n | 32767 |
565+
| test.c:602:9:602:9 | n | 32767 |
535566
| test.cpp:10:7:10:7 | b | 2147483647 |
536567
| test.cpp:11:5:11:5 | x | 2147483647 |
537568
| test.cpp:13:10:13:10 | x | 2147483647 |

‎python/ql/src/experimental/Customizations.qll‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/**
2+
* Contains customizations to the standard library.
3+
*
4+
* This module is imported by `python.qll`, so any customizations defined here automatically
5+
* apply to all queries.
6+
*
7+
* Typical examples of customizations include adding new subclasses of abstract classes such as
8+
* the `RemoteFlowSource::Range` and `AdditionalTaintStep` classes associated with the security
9+
* queries to model frameworks that are not covered by the standard library.
10+
*/
11+
12+
import python
13+
/* General import that is useful */
14+
// import experimental.dataflow.DataFlow
15+
//
16+
/* for extending `TaintTracking::AdditionalTaintStep` */
17+
// import experimental.dataflow.TaintTracking
18+
//
19+
/* for extending `RemoteFlowSource::Range` */
20+
// import experimental.dataflow.RemoteFlowSources

‎python/ql/src/experimental/dataflow/RemoteFlowSources.qll‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
private import python
2+
private import experimental.dataflow.DataFlow
3+
// Need to import since frameworks can extend `RemoteFlowSource::Range`
4+
private import experimental.semmle.python.Frameworks
5+
6+
/**
7+
* A data flow source of remote user input.
8+
*
9+
* Extend this class to refine existing API models. If you want to model new APIs,
10+
* extend `RemoteFlowSource::Range` instead.
11+
*/
12+
class RemoteFlowSource extends DataFlow::Node {
13+
RemoteFlowSource::Range self;
14+
15+
RemoteFlowSource() { this = self }
16+
17+
/** Gets a string that describes the type of this remote flow source. */
18+
string getSourceType() { result = self.getSourceType() }
19+
}
20+
21+
/** Provides a class for modeling new sources of remote user input. */
22+
module RemoteFlowSource {
23+
/**
24+
* A data flow source of remote user input.
25+
*
26+
* Extend this class to model new APIs. If you want to refine existing API models,
27+
* extend `RemoteFlowSource` instead.
28+
*/
29+
abstract class Range extends DataFlow::Node {
30+
/** Gets a string that describes the type of this remote flow source. */
31+
abstract string getSourceType();
32+
}
33+
}

‎python/ql/src/experimental/dataflow/TypeTracker.qll‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
/** Step Summaries and Type Tracking */
22

3-
import python
4-
import internal.DataFlowPublic
5-
import internal.DataFlowPrivate
3+
private import python
4+
private import internal.DataFlowPublic
5+
private import internal.DataFlowPrivate
66

77
/** Any string that may appear as the name of an attribute or access path. */
88
class AttributeName extends string {
@@ -144,7 +144,7 @@ private newtype TTypeTracker = MkTypeTracker(Boolean hasCall, OptionalAttributeN
144144
* It is recommended that all uses of this type are written in the following form,
145145
* for tracking some type `myType`:
146146
* ```
147-
* Node myType(DataFlow::TypeTracker t) {
147+
* DataFlow::Node myType(DataFlow::TypeTracker t) {
148148
* t.start() and
149149
* result = < source of myType >
150150
* or
@@ -153,7 +153,7 @@ private newtype TTypeTracker = MkTypeTracker(Boolean hasCall, OptionalAttributeN
153153
* )
154154
* }
155155
*
156-
* DataFlow::SourceNode myType() { result = myType(DataFlow::TypeTracker::end()) }
156+
* DataFlow::Node myType() { result = myType(DataFlow::TypeTracker::end()) }
157157
* ```
158158
*
159159
* Instead of `result = myType(t2).track(t2, t)`, you can also use the equivalent
@@ -280,3 +280,10 @@ class TypeTracker extends TTypeTracker {
280280
result = this
281281
}
282282
}
283+
284+
module TypeTracker {
285+
/**
286+
* Gets a valid end point of type tracking.
287+
*/
288+
TypeTracker end() { result.end() }
289+
}

‎python/ql/src/experimental/dataflow/internal/TaintTrackingPublic.qll‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
private import python
77
private import TaintTrackingPrivate
88
private import experimental.dataflow.DataFlow
9+
// Need to import since frameworks can extend `AdditionalTaintStep`
10+
private import experimental.semmle.python.Frameworks
911

1012
// Local taint flow and helpers
1113
/**

0 commit comments

Comments
 (0)