Skip to content

Commit 7e7dfeb

Browse files
authored
Merge branch 'main' into openssl_keyagreement_instances_and_consumers
2 parents 8b770bf + 52aa7e3 commit 7e7dfeb

File tree

8 files changed

+158
-75
lines changed

8 files changed

+158
-75
lines changed

.github/workflows/build-ripunzip.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,18 @@ on:
66
ripunzip-version:
77
description: "what reference to checktout from google/runzip"
88
required: false
9-
default: v1.2.1
9+
default: v2.0.2
1010
openssl-version:
1111
description: "what reference to checkout from openssl/openssl for Linux"
1212
required: false
13-
default: openssl-3.3.0
13+
default: openssl-3.5.0
1414

1515
jobs:
1616
build:
1717
strategy:
1818
fail-fast: false
1919
matrix:
20-
os: [ubuntu-22.04, macos-13, windows-2019]
20+
os: [ubuntu-22.04, macos-13, windows-2022]
2121
runs-on: ${{ matrix.os }}
2222
steps:
2323
- uses: actions/checkout@v4

.github/workflows/csharp-qltest.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ jobs:
3636
unit-tests:
3737
strategy:
3838
matrix:
39-
os: [ubuntu-latest, windows-2019]
39+
os: [ubuntu-latest, windows-latest]
4040
runs-on: ${{ matrix.os }}
4141
steps:
4242
- uses: actions/checkout@v4
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added taint flow through the `URL` constructor from the `url` package, improving the identification of SSRF vulnerabilities.

javascript/ql/lib/semmle/javascript/security/dataflow/RequestForgeryCustomizations.qll

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,13 @@ module RequestForgery {
8282
pred = url.getArgument(0)
8383
)
8484
or
85+
exists(DataFlow::NewNode url |
86+
url = API::moduleImport("url").getMember("URL").getAnInstantiation()
87+
|
88+
succ = url and
89+
pred = url.getArgument(0)
90+
)
91+
or
8592
exists(HtmlSanitizerCall call |
8693
pred = call.getInput() and
8794
succ = call

javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@
3030
| serverSide.js:117:20:117:30 | new ws(url) | serverSide.js:115:25:115:35 | request.url | serverSide.js:117:27:117:29 | url | The $@ of this request depends on a $@. | serverSide.js:117:27:117:29 | url | URL | serverSide.js:115:25:115:35 | request.url | user-provided value |
3131
| serverSide.js:125:5:128:6 | axios({ ... \\n }) | serverSide.js:123:29:123:35 | req.url | serverSide.js:127:14:127:20 | tainted | The $@ of this request depends on a $@. | serverSide.js:127:14:127:20 | tainted | URL | serverSide.js:123:29:123:35 | req.url | user-provided value |
3232
| serverSide.js:131:5:131:20 | axios.get(myUrl) | serverSide.js:123:29:123:35 | req.url | serverSide.js:131:15:131:19 | myUrl | The $@ of this request depends on a $@. | serverSide.js:131:15:131:19 | myUrl | URL | serverSide.js:123:29:123:35 | req.url | user-provided value |
33+
| serverSide.js:141:3:141:30 | axios.g ... ring()) | serverSide.js:139:17:139:29 | req.query.url | serverSide.js:141:13:141:29 | target.toString() | The $@ of this request depends on a $@. | serverSide.js:141:13:141:29 | target.toString() | URL | serverSide.js:139:17:139:29 | req.query.url | user-provided value |
34+
| serverSide.js:142:3:142:19 | axios.get(target) | serverSide.js:139:17:139:29 | req.query.url | serverSide.js:142:13:142:18 | target | The $@ of this request depends on a $@. | serverSide.js:142:13:142:18 | target | URL | serverSide.js:139:17:139:29 | req.query.url | user-provided value |
35+
| serverSide.js:143:3:143:24 | axios.g ... t.href) | serverSide.js:139:17:139:29 | req.query.url | serverSide.js:143:13:143:23 | target.href | The $@ of this request depends on a $@. | serverSide.js:143:13:143:23 | target.href | URL | serverSide.js:139:17:139:29 | req.query.url | user-provided value |
3336
edges
3437
| Request/app/api/proxy/route2.serverSide.ts:4:9:4:15 | { url } | Request/app/api/proxy/route2.serverSide.ts:4:9:4:34 | url | provenance | |
3538
| Request/app/api/proxy/route2.serverSide.ts:4:9:4:34 | url | Request/app/api/proxy/route2.serverSide.ts:5:27:5:29 | url | provenance | |
@@ -106,6 +109,15 @@ edges
106109
| serverSide.js:123:29:123:35 | req.url | serverSide.js:123:19:123:42 | url.par ... , true) | provenance | |
107110
| serverSide.js:130:9:130:45 | myUrl | serverSide.js:131:15:131:19 | myUrl | provenance | |
108111
| serverSide.js:130:37:130:43 | tainted | serverSide.js:130:9:130:45 | myUrl | provenance | |
112+
| serverSide.js:139:9:139:29 | input | serverSide.js:140:26:140:30 | input | provenance | |
113+
| serverSide.js:139:17:139:29 | req.query.url | serverSide.js:139:9:139:29 | input | provenance | |
114+
| serverSide.js:140:9:140:31 | target | serverSide.js:141:13:141:18 | target | provenance | |
115+
| serverSide.js:140:9:140:31 | target | serverSide.js:142:13:142:18 | target | provenance | |
116+
| serverSide.js:140:9:140:31 | target | serverSide.js:143:13:143:18 | target | provenance | |
117+
| serverSide.js:140:18:140:31 | new URL(input) | serverSide.js:140:9:140:31 | target | provenance | |
118+
| serverSide.js:140:26:140:30 | input | serverSide.js:140:18:140:31 | new URL(input) | provenance | Config |
119+
| serverSide.js:141:13:141:18 | target | serverSide.js:141:13:141:29 | target.toString() | provenance | |
120+
| serverSide.js:143:13:143:18 | target | serverSide.js:143:13:143:23 | target.href | provenance | |
109121
nodes
110122
| Request/app/api/proxy/route2.serverSide.ts:4:9:4:15 | { url } | semmle.label | { url } |
111123
| Request/app/api/proxy/route2.serverSide.ts:4:9:4:34 | url | semmle.label | url |
@@ -199,4 +211,14 @@ nodes
199211
| serverSide.js:130:9:130:45 | myUrl | semmle.label | myUrl |
200212
| serverSide.js:130:37:130:43 | tainted | semmle.label | tainted |
201213
| serverSide.js:131:15:131:19 | myUrl | semmle.label | myUrl |
214+
| serverSide.js:139:9:139:29 | input | semmle.label | input |
215+
| serverSide.js:139:17:139:29 | req.query.url | semmle.label | req.query.url |
216+
| serverSide.js:140:9:140:31 | target | semmle.label | target |
217+
| serverSide.js:140:18:140:31 | new URL(input) | semmle.label | new URL(input) |
218+
| serverSide.js:140:26:140:30 | input | semmle.label | input |
219+
| serverSide.js:141:13:141:18 | target | semmle.label | target |
220+
| serverSide.js:141:13:141:29 | target.toString() | semmle.label | target.toString() |
221+
| serverSide.js:142:13:142:18 | target | semmle.label | target |
222+
| serverSide.js:143:13:143:18 | target | semmle.label | target |
223+
| serverSide.js:143:13:143:23 | target.href | semmle.label | target.href |
202224
subpaths

javascript/ql/test/query-tests/Security/CWE-918/serverSide.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,3 +133,12 @@ var server2 = http.createServer(function(req, res) {
133133
var myEncodedUrl = `${something}/bla/${encodeURIComponent(tainted)}`;
134134
axios.get(myEncodedUrl);
135135
})
136+
137+
var server2 = http.createServer(function(req, res) {
138+
const { URL } = require('url');
139+
const input = req.query.url; // $Source[js/request-forgery]
140+
const target = new URL(input);
141+
axios.get(target.toString()); // $Alert[js/request-forgery]
142+
axios.get(target); // $Alert[js/request-forgery]
143+
axios.get(target.href); // $Alert[js/request-forgery]
144+
});

rust/ql/lib/codeql/rust/internal/TypeInference.qll

Lines changed: 43 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -207,81 +207,58 @@ private Type inferAssignmentOperationType(AstNode n, TypePath path) {
207207
}
208208

209209
/**
210-
* Holds if the type of `n1` at `path1` is the same as the type of `n2` at
211-
* `path2` and type information should propagate in both directions through the
212-
* type equality.
210+
* Holds if the type tree of `n1` at `prefix1` should be equal to the type tree
211+
* of `n2` at `prefix2` and type information should propagate in both directions
212+
* through the type equality.
213213
*/
214-
bindingset[path1]
215-
bindingset[path2]
216-
private predicate typeEquality(AstNode n1, TypePath path1, AstNode n2, TypePath path2) {
217-
exists(Variable v |
218-
path1 = path2 and
219-
n1 = v.getAnAccess()
220-
|
221-
n2 = v.getPat()
214+
private predicate typeEquality(AstNode n1, TypePath prefix1, AstNode n2, TypePath prefix2) {
215+
prefix1.isEmpty() and
216+
prefix2.isEmpty() and
217+
(
218+
exists(Variable v | n1 = v.getAnAccess() |
219+
n2 = v.getPat()
220+
or
221+
n2 = v.getParameter().(SelfParam)
222+
)
222223
or
223-
n2 = v.getParameter().(SelfParam)
224-
)
225-
or
226-
exists(LetStmt let |
227-
let.getPat() = n1 and
228-
let.getInitializer() = n2 and
229-
path1 = path2
230-
)
231-
or
232-
n1 = n2.(ParenExpr).getExpr() and
233-
path1 = path2
234-
or
235-
n1 = n2.(BlockExpr).getStmtList().getTailExpr() and
236-
path1 = path2
237-
or
238-
n1 = n2.(IfExpr).getABranch() and
239-
path1 = path2
240-
or
241-
n1 = n2.(MatchExpr).getAnArm().getExpr() and
242-
path1 = path2
243-
or
244-
exists(BreakExpr break |
245-
break.getExpr() = n1 and
246-
break.getTarget() = n2.(LoopExpr) and
247-
path1 = path2
248-
)
249-
or
250-
exists(AssignmentExpr be |
251-
n1 = be.getLhs() and
252-
n2 = be.getRhs() and
253-
path1 = path2
254-
)
255-
}
256-
257-
bindingset[path1]
258-
private predicate typeEqualityLeft(AstNode n1, TypePath path1, AstNode n2, TypePath path2) {
259-
typeEquality(n1, path1, n2, path2)
260-
or
261-
n2 =
262-
any(DerefExpr pe |
263-
pe.getExpr() = n1 and
264-
path1.isCons(TRefTypeParameter(), path2)
224+
exists(LetStmt let |
225+
let.getPat() = n1 and
226+
let.getInitializer() = n2
265227
)
266-
}
267-
268-
bindingset[path2]
269-
private predicate typeEqualityRight(AstNode n1, TypePath path1, AstNode n2, TypePath path2) {
270-
typeEquality(n1, path1, n2, path2)
271-
or
272-
n2 =
273-
any(DerefExpr pe |
274-
pe.getExpr() = n1 and
275-
path1 = TypePath::cons(TRefTypeParameter(), path2)
228+
or
229+
n1 = n2.(ParenExpr).getExpr()
230+
or
231+
n1 = n2.(BlockExpr).getStmtList().getTailExpr()
232+
or
233+
n1 = n2.(IfExpr).getABranch()
234+
or
235+
n1 = n2.(MatchExpr).getAnArm().getExpr()
236+
or
237+
exists(BreakExpr break |
238+
break.getExpr() = n1 and
239+
break.getTarget() = n2.(LoopExpr)
240+
)
241+
or
242+
exists(AssignmentExpr be |
243+
n1 = be.getLhs() and
244+
n2 = be.getRhs()
276245
)
246+
)
247+
or
248+
n1 = n2.(DerefExpr).getExpr() and
249+
prefix1 = TypePath::singleton(TRefTypeParameter()) and
250+
prefix2.isEmpty()
277251
}
278252

279253
pragma[nomagic]
280254
private Type inferTypeEquality(AstNode n, TypePath path) {
281-
exists(AstNode n2, TypePath path2 | result = inferType(n2, path2) |
282-
typeEqualityRight(n, path, n2, path2)
255+
exists(TypePath prefix1, AstNode n2, TypePath prefix2, TypePath suffix |
256+
result = inferType(n2, prefix2.appendInverse(suffix)) and
257+
path = prefix1.append(suffix)
258+
|
259+
typeEquality(n, prefix1, n2, prefix2)
283260
or
284-
typeEqualityLeft(n2, path2, n, path)
261+
typeEquality(n2, prefix2, n, prefix1)
285262
)
286263
}
287264

shared/quantum/codeql/quantum/experimental/Model.qll

Lines changed: 69 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,17 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
424424
final override ConsumerInputDataFlowNode getInputNode() { result = inputNode }
425425
}
426426

427+
final private class SignatureArtifactConsumer extends ArtifactConsumerAndInstance {
428+
ConsumerInputDataFlowNode inputNode;
429+
430+
SignatureArtifactConsumer() {
431+
exists(SignatureOperationInstance op | inputNode = op.getSignatureConsumer()) and
432+
this = Input::dfn_to_element(inputNode)
433+
}
434+
435+
final override ConsumerInputDataFlowNode getInputNode() { result = inputNode }
436+
}
437+
427438
/**
428439
* An artifact that is produced by an operation, representing a concrete artifact instance rather than a synthetic consumer artifact.
429440
*/
@@ -458,6 +469,8 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
458469
}
459470

460471
override DataFlowNode getOutputNode() { result = creator.getOutputArtifact() }
472+
473+
KeyOperationInstance getCreator() { result = creator }
461474
}
462475

463476
/**
@@ -782,6 +795,17 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
782795
abstract ArtifactOutputDataFlowNode getOutputArtifact();
783796
}
784797

798+
/**
799+
* A key operation instance representing a signature being generated or verified.
800+
*/
801+
abstract class SignatureOperationInstance extends KeyOperationInstance {
802+
/**
803+
* Gets the consumer of the signature that is being verified in case of a
804+
* verification operation.
805+
*/
806+
abstract ConsumerInputDataFlowNode getSignatureConsumer();
807+
}
808+
785809
/**
786810
* A key-based algorithm instance used in cryptographic operations such as encryption, decryption,
787811
* signing, verification, and key wrapping.
@@ -1266,6 +1290,7 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
12661290
TNonceInput(NonceArtifactConsumer e) or
12671291
TMessageInput(MessageArtifactConsumer e) or
12681292
TSaltInput(SaltArtifactConsumer e) or
1293+
TSignatureInput(SignatureArtifactConsumer e) or
12691294
TRandomNumberGeneration(RandomNumberGenerationInstance e) { e.flowsTo(_) } or
12701295
// Key Creation Operation union type (e.g., key generation, key load)
12711296
TKeyCreationOperation(KeyCreationOperationInstance e) or
@@ -1327,14 +1352,14 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
13271352
/**
13281353
* Returns the child of this node with the given edge name.
13291354
*
1330-
* This predicate is overriden by derived classes to construct the graph of cryptographic operations.
1355+
* This predicate is overridden by derived classes to construct the graph of cryptographic operations.
13311356
*/
13321357
NodeBase getChild(string edgeName) { none() }
13331358

13341359
/**
13351360
* Defines properties of this node by name and either a value or location or both.
13361361
*
1337-
* This predicate is overriden by derived classes to construct the graph of cryptographic operations.
1362+
* This predicate is overridden by derived classes to construct the graph of cryptographic operations.
13381363
*/
13391364
predicate properties(string key, string value, Location location) { none() }
13401365

@@ -1507,6 +1532,20 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
15071532
override LocatableElement asElement() { result = instance }
15081533
}
15091534

1535+
/**
1536+
* A signature input. This may represent a signature, or a signature component
1537+
* such as the scalar values r and s in ECDSA.
1538+
*/
1539+
final class SignatureArtifactNode extends ArtifactNode, TSignatureInput {
1540+
SignatureArtifactConsumer instance;
1541+
1542+
SignatureArtifactNode() { this = TSignatureInput(instance) }
1543+
1544+
final override string getInternalType() { result = "SignatureInput" }
1545+
1546+
override LocatableElement asElement() { result = instance }
1547+
}
1548+
15101549
/**
15111550
* A salt input.
15121551
*/
@@ -1530,13 +1569,22 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
15301569

15311570
KeyOperationOutputNode() { this = TKeyOperationOutput(instance) }
15321571

1533-
final override string getInternalType() { result = "KeyOperationOutput" }
1572+
override string getInternalType() { result = "KeyOperationOutput" }
15341573

15351574
override LocatableElement asElement() { result = instance }
15361575

15371576
override string getSourceNodeRelationship() { none() }
15381577
}
15391578

1579+
class SignOperationOutputNode extends KeyOperationOutputNode {
1580+
SignOperationOutputNode() {
1581+
this.asElement().(KeyOperationOutputArtifactInstance).getCreator().getKeyOperationSubtype() =
1582+
TSignMode()
1583+
}
1584+
1585+
override string getInternalType() { result = "SignatureOutput" }
1586+
}
1587+
15401588
/**
15411589
* A source of random number generation.
15421590
*/
@@ -2109,6 +2157,7 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
21092157
}
21102158

21112159
class SignatureOperationNode extends KeyOperationNode {
2160+
override SignatureOperationInstance instance;
21122161
string nodeName;
21132162

21142163
SignatureOperationNode() {
@@ -2118,6 +2167,21 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
21182167
}
21192168

21202169
override string getInternalType() { result = nodeName }
2170+
2171+
SignatureArtifactNode getASignatureArtifact() {
2172+
result.asElement() = instance.getSignatureConsumer().getConsumer()
2173+
}
2174+
2175+
override NodeBase getChild(string key) {
2176+
result = super.getChild(key)
2177+
or
2178+
// [KNOWN_OR_UNKNOWN] - only if we know the type is verify
2179+
this.getKeyOperationSubtype() = TVerifyMode() and
2180+
key = "Signature" and
2181+
if exists(this.getASignatureArtifact())
2182+
then result = this.getASignatureArtifact()
2183+
else result = this
2184+
}
21212185
}
21222186

21232187
/**
@@ -2565,15 +2629,15 @@ module CryptographyBase<LocationSig Location, InputSig<Location> Input> {
25652629
or
25662630
curveName = "CURVE25519" and keySize = 255 and curveFamily = CURVE25519()
25672631
or
2632+
curveName = "CURVE448" and keySize = 448 and curveFamily = CURVE448()
2633+
or
25682634
// TODO: separate these into key agreement logic or sign/verify (ECDSA / ECDH)
25692635
// or
25702636
// curveName = "X25519" and keySize = 255 and curveFamily = CURVE25519()
25712637
// or
25722638
// curveName = "ED25519" and keySize = 255 and curveFamily = CURVE25519()
25732639
// or
25742640
// curveName = "ED448" and keySize = 448 and curveFamily = CURVE448()
2575-
// curveName = "CURVE448" and keySize = 448 and curveFamily = CURVE448()
2576-
// or
25772641
// or
25782642
// curveName = "X448" and keySize = 448 and curveFamily = CURVE448()
25792643
curveName = "SM2" and keySize in [256, 512] and curveFamily = SM2()

0 commit comments

Comments
 (0)