Skip to content

Commit 5b28ff1

Browse files
authored
Merge branch 'main' into aibaars/rust-extract-libs
2 parents c8ff69a + 0822ded commit 5b28ff1

File tree

82 files changed

+1939
-1145
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

82 files changed

+1939
-1145
lines changed

.gitignore

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,3 +72,7 @@ node_modules/
7272

7373
# cargo build directory
7474
/target
75+
76+
# some upgrade/downgrade checks create these files
77+
**/upgrades/*/*.dbscheme.stats
78+
**/downgrades/*/*.dbscheme.stats

actions/ql/lib/ext/config/actions_permissions.yml

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,21 @@ extensions:
2222
- ["actions/stale", "pull-requests: write"]
2323
- ["actions/attest-build-provenance", "id-token: write"]
2424
- ["actions/attest-build-provenance", "attestations: write"]
25+
- ["actions/deploy-pages", "pages: write"]
26+
- ["actions/deploy-pages", "id-token: write"]
27+
- ["actions/delete-package-versions", "packages: write"]
2528
- ["actions/jekyll-build-pages", "contents: read"]
2629
- ["actions/jekyll-build-pages", "pages: write"]
2730
- ["actions/jekyll-build-pages", "id-token: write"]
2831
- ["actions/publish-action", "contents: write"]
29-
- ["actions/versions-package-tools", "contents: read"]
32+
- ["actions/versions-package-tools", "contents: read"]
3033
- ["actions/versions-package-tools", "actions: read"]
31-
- ["actions/reusable-workflows", "contents: read"]
34+
- ["actions/reusable-workflows", "contents: read"]
3235
- ["actions/reusable-workflows", "actions: read"]
36+
- ["actions/ai-inference", "contents: read"]
37+
- ["actions/ai-inference", "models: read"]
3338
# TODO: Add permissions for actions/download-artifact
3439
# TODO: Add permissions for actions/upload-artifact
40+
# No permissions needed for actions/upload-pages-artifact
3541
# TODO: Add permissions for actions/cache
36-
37-
42+
# No permissions needed for actions/configure-pages
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The query `actions/missing-workflow-permissions` is now aware of the minimal permissions needed for the actions `deploy-pages`, `delete-package-versions`, `ai-inference`. This should lead to better alert messages and better fix suggestions.
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
on:
2+
workflow_call:
3+
workflow_dispatch:
4+
5+
jobs:
6+
build:
7+
name: Build and test
8+
runs-on: ubuntu-latest
9+
steps:
10+
- uses: actions/ai-inference
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
on:
2+
workflow_call:
3+
workflow_dispatch:
4+
5+
jobs:
6+
build:
7+
name: Build and test
8+
runs-on: ubuntu-latest
9+
steps:
10+
- uses: actions/deploy-pages
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
on:
2+
workflow_call:
3+
workflow_dispatch:
4+
5+
jobs:
6+
build:
7+
name: Build and test
8+
runs-on: ubuntu-latest
9+
steps:
10+
- uses: actions/delete-package-versions

actions/ql/test/query-tests/Security/CWE-275/MissingActionsPermissions.expected

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,6 @@
33
| .github/workflows/perms5.yml:7:5:10:32 | Job: build | Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {contents: read} |
44
| .github/workflows/perms6.yml:7:5:11:39 | Job: build | Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {contents: read, id-token: write, pages: write} |
55
| .github/workflows/perms7.yml:7:5:10:38 | Job: build | Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {} |
6+
| .github/workflows/perms8.yml:7:5:10:33 | Job: build | Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {id-token: write, pages: write} |
7+
| .github/workflows/perms9.yml:7:5:10:44 | Job: build | Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {packages: write} |
8+
| .github/workflows/perms10.yml:7:5:10:33 | Job: build | Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {contents: read, models: read} |

cpp/ql/lib/experimental/quantum/Language.qll

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
private import cpp as Language
22
import semmle.code.cpp.dataflow.new.TaintTracking
33
import codeql.quantum.experimental.Model
4+
private import OpenSSL.GenericSourceCandidateLiteral
45

56
module CryptoInput implements InputSig<Language::Location> {
67
class DataFlowNode = DataFlow::Node;
@@ -89,16 +90,7 @@ module GenericDataSourceFlowConfig implements DataFlow::ConfigSig {
8990
module GenericDataSourceFlow = TaintTracking::Global<GenericDataSourceFlowConfig>;
9091

9192
private class ConstantDataSource extends Crypto::GenericConstantSourceInstance instanceof Literal {
92-
ConstantDataSource() {
93-
// TODO: this is an API specific workaround for OpenSSL, as 'EC' is a constant that may be used
94-
// where typical algorithms are specified, but EC specifically means set up a
95-
// default curve container, that will later be specified explicitly (or if not a default)
96-
// curve is used.
97-
this.getValue() != "EC" and
98-
// Exclude all 0's as algorithms. Currently we know of no algorithm defined as 0, and
99-
// the typical case is 0 is assigned to represent null.
100-
this.getValue().toInt() != 0
101-
}
93+
ConstantDataSource() { this instanceof OpenSSLGenericSourceCandidateLiteral }
10294

10395
override DataFlow::Node getOutputNode() { result.asExpr() = this }
10496

cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/KnownAlgorithmConstants.qll

Lines changed: 24 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import cpp
2+
import experimental.quantum.OpenSSL.GenericSourceCandidateLiteral
23

34
predicate resolveAlgorithmFromExpr(Expr e, string normalizedName, string algType) {
45
resolveAlgorithmFromCall(e, normalizedName, algType)
@@ -32,30 +33,20 @@ class KnownOpenSSLCipherAlgorithmConstant extends KnownOpenSSLAlgorithmConstant
3233
}
3334

3435
class KnownOpenSSLPaddingAlgorithmConstant extends KnownOpenSSLAlgorithmConstant {
35-
string algType;
36-
3736
KnownOpenSSLPaddingAlgorithmConstant() {
38-
resolveAlgorithmFromExpr(this, _, algType) and
39-
algType.matches("%PADDING")
37+
exists(string algType |
38+
resolveAlgorithmFromExpr(this, _, algType) and
39+
algType.matches("%PADDING")
40+
)
4041
}
4142
}
4243

4344
class KnownOpenSSLBlockModeAlgorithmConstant extends KnownOpenSSLAlgorithmConstant {
44-
string algType;
45-
46-
KnownOpenSSLBlockModeAlgorithmConstant() {
47-
resolveAlgorithmFromExpr(this, _, algType) and
48-
algType.matches("%BLOCK_MODE")
49-
}
45+
KnownOpenSSLBlockModeAlgorithmConstant() { resolveAlgorithmFromExpr(this, _, "BLOCK_MODE") }
5046
}
5147

5248
class KnownOpenSSLHashAlgorithmConstant extends KnownOpenSSLAlgorithmConstant {
53-
string algType;
54-
55-
KnownOpenSSLHashAlgorithmConstant() {
56-
resolveAlgorithmFromExpr(this, _, algType) and
57-
algType.matches("%HASH")
58-
}
49+
KnownOpenSSLHashAlgorithmConstant() { resolveAlgorithmFromExpr(this, _, "HASH") }
5950

6051
int getExplicitDigestLength() {
6152
exists(string name |
@@ -68,13 +59,14 @@ class KnownOpenSSLHashAlgorithmConstant extends KnownOpenSSLAlgorithmConstant {
6859

6960
class KnownOpenSSLEllipticCurveAlgorithmConstant extends KnownOpenSSLAlgorithmConstant {
7061
KnownOpenSSLEllipticCurveAlgorithmConstant() {
71-
exists(string algType |
72-
resolveAlgorithmFromExpr(this, _, algType) and
73-
algType.matches("ELLIPTIC_CURVE")
74-
)
62+
resolveAlgorithmFromExpr(this, _, "ELLIPTIC_CURVE")
7563
}
7664
}
7765

66+
class KnownOpenSSLSignatureAlgorithmConstant extends KnownOpenSSLAlgorithmConstant {
67+
KnownOpenSSLSignatureAlgorithmConstant() { resolveAlgorithmFromExpr(this, _, "SIGNATURE") }
68+
}
69+
7870
/**
7971
* Resolves a call to a 'direct algorithm getter', e.g., EVP_MD5()
8072
* This approach to fetching algorithms was used in OpenSSL 1.0.2.
@@ -101,10 +93,10 @@ predicate resolveAlgorithmFromCall(Call c, string normalized, string algType) {
10193
* if `e` resolves to a known algorithm.
10294
* If this predicate does not hold, then `e` can be interpreted as being of `UNKNOWN` type.
10395
*/
104-
predicate resolveAlgorithmFromLiteral(Literal e, string normalized, string algType) {
105-
exists(int nid |
106-
nid = getPossibleNidFromLiteral(e) and knownOpenSSLAlgorithmLiteral(_, nid, normalized, algType)
107-
)
96+
predicate resolveAlgorithmFromLiteral(
97+
OpenSSLGenericSourceCandidateLiteral e, string normalized, string algType
98+
) {
99+
knownOpenSSLAlgorithmLiteral(_, e.getValue().toInt(), normalized, algType)
108100
or
109101
exists(string name |
110102
name = resolveAlgorithmAlias(e.getValue()) and
@@ -123,30 +115,6 @@ string resolveAlgorithmAlias(string name) {
123115
)
124116
}
125117

126-
private int getPossibleNidFromLiteral(Literal e) {
127-
result = e.getValue().toInt() and
128-
not e instanceof CharLiteral and
129-
not e instanceof StringLiteral and
130-
// ASSUMPTION, no negative numbers are allowed
131-
// RATIONALE: this is a performance improvement to avoid having to trace every number
132-
not exists(UnaryMinusExpr u | u.getOperand() = e) and
133-
// OPENSSL has a special macro for getting every line, ignore it
134-
not exists(MacroInvocation mi | mi.getExpr() = e and mi.getMacroName() = "OPENSSL_LINE") and
135-
// Filter out cases where an int is assigned into a pointer, e.g., char* x = NULL;
136-
not exists(Assignment a |
137-
a.getRValue() = e and a.getLValue().getType().getUnspecifiedType() instanceof PointerType
138-
) and
139-
not exists(Initializer i |
140-
i.getExpr() = e and
141-
i.getDeclaration().getADeclarationEntry().getUnspecifiedType() instanceof PointerType
142-
) and
143-
// Filter out cases where an int is returned into a pointer, e.g., return NULL;
144-
not exists(ReturnStmt r |
145-
r.getExpr() = e and
146-
r.getEnclosingFunction().getType().getUnspecifiedType() instanceof PointerType
147-
)
148-
}
149-
150118
string getAlgorithmAlias(string alias) {
151119
customAliases(result, alias)
152120
or
@@ -260,11 +228,6 @@ predicate defaultAliases(string target, string alias) {
260228
alias = "ssl3-sha1" and target = "sha1"
261229
}
262230

263-
predicate tbd(string normalized, string algType) {
264-
knownOpenSSLAlgorithmLiteral(_, _, normalized, algType) and
265-
algType = "HASH"
266-
}
267-
268231
/**
269232
* Enumeration of all known crypto algorithms for openSSL
270233
* `name` is all lower case (caller's must ensure they pass in lower case)
@@ -291,8 +254,12 @@ predicate knownOpenSSLAlgorithmLiteral(string name, int nid, string normalized,
291254
or
292255
name = "ed25519" and nid = 1087 and normalized = "ED25519" and algType = "ELLIPTIC_CURVE"
293256
or
257+
name = "ed25519" and nid = 1087 and normalized = "ED25519" and algType = "SIGNATURE"
258+
or
294259
name = "ed448" and nid = 1088 and normalized = "ED448" and algType = "ELLIPTIC_CURVE"
295260
or
261+
name = "ed448" and nid = 1088 and normalized = "ED448" and algType = "SIGNATURE"
262+
or
296263
name = "md2" and nid = 3 and normalized = "MD2" and algType = "HASH"
297264
or
298265
name = "sha" and nid = 41 and normalized = "SHA" and algType = "HASH"
@@ -1712,8 +1679,12 @@ predicate knownOpenSSLAlgorithmLiteral(string name, int nid, string normalized,
17121679
or
17131680
name = "x448" and nid = 1035 and normalized = "X448" and algType = "ELLIPTIC_CURVE"
17141681
or
1682+
name = "x448" and nid = 1035 and normalized = "X448" and algType = "KEY_EXCHANGE"
1683+
or
17151684
name = "x25519" and nid = 1034 and normalized = "X25519" and algType = "ELLIPTIC_CURVE"
17161685
or
1686+
name = "x25519" and nid = 1034 and normalized = "X25519" and algType = "KEY_EXCHANGE"
1687+
or
17171688
name = "authecdsa" and nid = 1047 and normalized = "ECDSA" and algType = "SIGNATURE"
17181689
or
17191690
name = "authgost01" and nid = 1050 and normalized = "GOST" and algType = "SYMMETRIC_ENCRYPTION"
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
import cpp
2+
private import semmle.code.cpp.models.Models
3+
private import semmle.code.cpp.models.interfaces.FormattingFunction
4+
5+
private class IntLiteral extends Literal {
6+
IntLiteral() {
7+
//Heuristics for distinguishing int literals from other literals
8+
exists(this.getValue().toInt()) and
9+
not this instanceof CharLiteral and
10+
not this instanceof StringLiteral
11+
}
12+
}
13+
14+
/**
15+
* Holds if a StringLiteral could conceivably be used in some way for cryptography.
16+
* Note: this predicate should only consider restrictions with respect to strings only.
17+
* General restrictions are in the OpenSSLGenericSourceCandidateLiteral class.
18+
*/
19+
private predicate isOpenSSLStringLiteralGenericSourceCandidate(StringLiteral s) {
20+
// 'EC' is a constant that may be used where typical algorithms are specified,
21+
// but EC specifically means set up a default curve container, that will later be
22+
//specified explicitly (or if not a default) curve is used.
23+
s.getValue() != "EC" and
24+
// Ignore empty strings
25+
s.getValue() != "" and
26+
// Filter out strings with "%", to filter out format strings
27+
not s.getValue().matches("%\\%%") and
28+
// Filter out strings in brackets or braces (commonly seen strings not relevant for crypto)
29+
not s.getValue().matches(["[%]", "(%)"]) and
30+
// Filter out all strings of length 1, since these are not algorithm names
31+
// NOTE/ASSUMPTION: If a user legitimately passes a string of length 1 to some configuration
32+
// we will assume this is generally unknown. We may need to reassess this in the future.
33+
s.getValue().length() > 1 and
34+
// Ignore all strings that are in format string calls outputing to a stream (e.g., stdout)
35+
not exists(FormattingFunctionCall f |
36+
exists(f.getOutputArgument(true)) and s = f.(Call).getAnArgument()
37+
) and
38+
// Ignore all format string calls where there is no known out param (resulting string)
39+
// i.e., ignore printf, since it will just output a string and not produce a new string
40+
not exists(FormattingFunctionCall f |
41+
// Note: using two ways of determining if there is an out param, since I'm not sure
42+
// which way is canonical
43+
not exists(f.getOutputArgument(false)) and
44+
not f.getTarget().hasTaintFlow(_, _) and
45+
f.(Call).getAnArgument() = s
46+
)
47+
}
48+
49+
/**
50+
* Holds if a StringLiteral could conceivably be used in some way for cryptography.
51+
* Note: this predicate should only consider restrictions with respect to integers only.
52+
* General restrictions are in the OpenSSLGenericSourceCandidateLiteral class.
53+
*/
54+
private predicate isOpenSSLIntLiteralGenericSourceCandidate(IntLiteral l) {
55+
// Ignore integer values of 0, commonly referring to NULL only (no known algorithm 0)
56+
l.getValue().toInt() != 0 and
57+
// ASSUMPTION, no negative numbers are allowed
58+
// RATIONALE: this is a performance improvement to avoid having to trace every number
59+
not exists(UnaryMinusExpr u | u.getOperand() = l) and
60+
// OPENSSL has a special macro for getting every line, ignore it
61+
not exists(MacroInvocation mi | mi.getExpr() = l and mi.getMacroName() = "OPENSSL_LINE") and
62+
// Filter out cases where an int is returned into a pointer, e.g., return NULL;
63+
not exists(ReturnStmt r |
64+
r.getExpr() = l and
65+
r.getEnclosingFunction().getType().getUnspecifiedType() instanceof DerivedType
66+
) and
67+
// A literal as an array index should not be relevant for crypo
68+
not exists(ArrayExpr op | op.getArrayOffset() = l) and
69+
// A literal used in a bitwise should not be relevant for crypto
70+
not exists(BinaryBitwiseOperation op | op.getAnOperand() = l) and
71+
not exists(AssignBitwiseOperation op | op.getAnOperand() = l) and
72+
//Filter out cases where an int is assigned or initialized into a pointer, e.g., char* x = NULL;
73+
not exists(Assignment a |
74+
a.getRValue() = l and
75+
a.getLValue().getType().getUnspecifiedType() instanceof DerivedType
76+
) and
77+
not exists(Initializer i |
78+
i.getExpr() = l and
79+
i.getDeclaration().getADeclarationEntry().getUnspecifiedType() instanceof DerivedType
80+
) and
81+
// Filter out cases where the literal is used in any kind of arithmetic operation
82+
not exists(BinaryArithmeticOperation op | op.getAnOperand() = l) and
83+
not exists(UnaryArithmeticOperation op | op.getOperand() = l) and
84+
not exists(AssignArithmeticOperation op | op.getAnOperand() = l) and
85+
// If a literal has no parent ignore it, this is a workaround for the current inability
86+
// to find a literal in an array declaration: int x[100];
87+
// NOTE/ASSUMPTION: this value might actually be relevant for finding hard coded sizes
88+
// consider size as inferred through the allocation of a buffer.
89+
// In these cases, we advise that the source is not generic and must be traced explicitly.
90+
exists(l.getParent())
91+
}
92+
93+
/**
94+
* Any literal that may be conceivably be used in some way for cryptography.
95+
* The set of all literals is restricted by this class to cases where there is higher
96+
* plausibility that the literal could be used as a source of configuration.
97+
* Literals are filtered, for example, if they are used in a way no indicative of an algorithm use
98+
* such as in an array index, bitwise operation, or logical operation.
99+
* Note a case like this:
100+
* if(algVal == "AES")
101+
*
102+
* "AES" may be a legitimate algorithm literal, but the literal will not be used for an operation directly
103+
* since it is in a equality comparison, hence this case would also be filtered.
104+
*/
105+
class OpenSSLGenericSourceCandidateLiteral extends Literal {
106+
OpenSSLGenericSourceCandidateLiteral() {
107+
(
108+
isOpenSSLIntLiteralGenericSourceCandidate(this) or
109+
isOpenSSLStringLiteralGenericSourceCandidate(this)
110+
) and
111+
// ********* General filters beyond what is filtered for strings and ints *********
112+
// An algorithm literal in a switch case will not be directly applied to an operation.
113+
not exists(SwitchCase sc | sc.getExpr() = this) and
114+
// A literal in a logical operation may be an algorithm, but not a candidate
115+
// for the purposes of finding applied algorithms
116+
not exists(BinaryLogicalOperation op | op.getAnOperand() = this) and
117+
not exists(UnaryLogicalOperation op | op.getOperand() = this) and
118+
// A literal in a comparison operation may be an algorithm, but not a candidate
119+
// for the purposes of finding applied algorithms
120+
not exists(ComparisonOperation op | op.getAnOperand() = this)
121+
}
122+
}

cpp/ql/lib/experimental/quantum/OpenSSL/OpenSSL.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,5 @@ module OpenSSLModel {
33
import AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
44
import Operations.OpenSSLOperations
55
import Random
6+
import GenericSourceCandidateLiteral
67
}

cpp/ql/lib/semmle/code/cpp/dataflow/internal/FlowSummaryImpl.qll

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,11 @@ module Input implements InputSig<Location, DataFlowImplSpecific::CppDataFlow> {
2222

2323
ArgumentPosition callbackSelfParameterPosition() { result = TDirectPosition(-1) }
2424

25-
ReturnKind getStandardReturnValueKind() { result.(NormalReturnKind).getIndirectionIndex() = 0 }
25+
ReturnKind getStandardReturnValueKind() { result = getReturnValueKind("") }
26+
27+
ReturnKind getReturnValueKind(string arg) {
28+
arg = repeatStars(result.(NormalReturnKind).getIndirectionIndex())
29+
}
2630

2731
string encodeParameterPosition(ParameterPosition pos) { result = pos.toString() }
2832

0 commit comments

Comments
 (0)