Skip to content

Commit 9f6e446

Browse files
authored
Merge branch 'main' into redsun82/rust-derive-macro-expansion
2 parents c22526e + 666144e commit 9f6e446

File tree

31 files changed

+556
-168
lines changed

31 files changed

+556
-168
lines changed

javascript/extractor/src/com/semmle/js/extractor/AutoBuild.java

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@
3939

4040
import com.google.gson.Gson;
4141
import com.google.gson.JsonParseException;
42+
import com.semmle.js.extractor.tsconfig.TsConfigJson;
43+
import com.semmle.js.extractor.tsconfig.CompilerOptions;
4244
import com.semmle.js.dependencies.AsyncFetcher;
4345
import com.semmle.js.dependencies.DependencyResolver;
4446
import com.semmle.js.dependencies.packument.PackageJson;
@@ -745,6 +747,26 @@ private CompletableFuture<?> extractSource() throws IOException {
745747
.filter(p -> !isFileTooLarge(p))
746748
.sorted(PATH_ORDERING)
747749
.collect(Collectors.toCollection(() -> new LinkedHashSet<>()));
750+
// gather all output directories specified in tsconfig.json files
751+
final List<Path> outDirs = new ArrayList<>();
752+
for (Path cfg : tsconfigFiles) {
753+
try {
754+
String txt = new WholeIO().read(cfg);
755+
TsConfigJson root = new Gson().fromJson(txt, TsConfigJson.class);
756+
if (root != null && root.getCompilerOptions() != null) {
757+
if (root.getCompilerOptions().getOutDir() == null) {
758+
// no outDir specified, so skip this tsconfig.json
759+
continue;
760+
}
761+
Path odir = cfg.getParent().resolve(root.getCompilerOptions().getOutDir()).toAbsolutePath().normalize();
762+
outDirs.add(odir);
763+
}
764+
} catch (Exception e) {
765+
// ignore malformed tsconfig or missing fields
766+
}
767+
}
768+
// exclude files in output directories as configured in tsconfig.json
769+
filesToExtract.removeIf(f -> outDirs.stream().anyMatch(od -> f.startsWith(od)));
748770

749771
DependencyInstallationResult dependencyInstallationResult = DependencyInstallationResult.empty;
750772
if (!tsconfigFiles.isEmpty()) {
@@ -796,9 +818,19 @@ private CompletableFuture<?> extractFiles(
796818
*/
797819
private boolean isFileDerivedFromTypeScriptFile(Path path, Set<Path> extractedFiles) {
798820
String name = path.getFileName().toString();
799-
if (!name.endsWith(".js"))
821+
// only skip JS variants when a corresponding TS/TSX file was already extracted
822+
if (!(name.endsWith(".js")
823+
|| name.endsWith(".cjs")
824+
|| name.endsWith(".mjs")
825+
|| name.endsWith(".jsx")
826+
|| name.endsWith(".cjsx")
827+
|| name.endsWith(".mjsx"))) {
800828
return false;
801-
String stem = name.substring(0, name.length() - ".js".length());
829+
}
830+
// strip off extension
831+
int dot = name.lastIndexOf('.');
832+
String stem = dot != -1 ? name.substring(0, dot) : name;
833+
// if a TS/TSX file with same base name was extracted, skip this file
802834
for (String ext : FileType.TYPESCRIPT.getExtensions()) {
803835
if (extractedFiles.contains(path.getParent().resolve(stem + ext))) {
804836
return true;
@@ -1154,7 +1186,7 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
11541186
}
11551187

11561188
// extract TypeScript projects from 'tsconfig.json'
1157-
if (typeScriptMode == TypeScriptMode.FULL
1189+
if (typeScriptMode != TypeScriptMode.NONE
11581190
&& treatAsTSConfig(file.getFileName().toString())
11591191
&& !excludes.contains(file)
11601192
&& isFileIncluded(file)) {
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package com.semmle.js.extractor.tsconfig;
2+
3+
public class CompilerOptions {
4+
private String outDir;
5+
6+
public String getOutDir() {
7+
return outDir;
8+
}
9+
10+
public void setOutDir(String outDir) {
11+
this.outDir = outDir;
12+
}
13+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package com.semmle.js.extractor.tsconfig;
2+
3+
public class TsConfigJson {
4+
private CompilerOptions compilerOptions;
5+
6+
public CompilerOptions getCompilerOptions() {
7+
return compilerOptions;
8+
}
9+
10+
public void setCompilerOptions(CompilerOptions compilerOptions) {
11+
this.compilerOptions = compilerOptions;
12+
}
13+
}

javascript/extractor/test/com/semmle/js/extractor/test/AutoBuildTests.java

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ public void extractTypeScriptFiles(
135135
FileExtractors extractors) {
136136
for (Path f : files) {
137137
actual.add(f.toString());
138+
extractedFiles.add(f);
138139
}
139140
}
140141

@@ -175,7 +176,7 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
175176

176177
@Test
177178
public void basicTest() throws IOException {
178-
addFile(true, LGTM_SRC, "tst.js");
179+
addFile(false, LGTM_SRC, "tst.js");
179180
addFile(true, LGTM_SRC, "tst.ts");
180181
addFile(true, LGTM_SRC, "tst.html");
181182
addFile(true, LGTM_SRC, "tst.xsjs");
@@ -203,6 +204,43 @@ public void typescriptWrongConfig() throws IOException {
203204
runTest();
204205
}
205206

207+
@Test
208+
public void skipJsFilesDerivedFromTypeScriptFiles() throws IOException {
209+
// JS-derived files (.js, .cjs, .mjs, .jsx, .cjsx, .mjsx) should be skipped when TS indexing
210+
envVars.put("LGTM_INDEX_TYPESCRIPT", "basic");
211+
// Add TypeScript sources
212+
addFile(true, LGTM_SRC, "foo.ts");
213+
addFile(true, LGTM_SRC, "bar.tsx");
214+
// Add derived JS variants (should be skipped)
215+
addFile(false, LGTM_SRC, "foo.js");
216+
addFile(false, LGTM_SRC, "bar.jsx");
217+
addFile(false, LGTM_SRC, "foo.cjs");
218+
addFile(false, LGTM_SRC, "foo.mjs");
219+
addFile(false, LGTM_SRC, "bar.cjsx");
220+
addFile(false, LGTM_SRC, "bar.mjsx");
221+
// A normal JS file without TS counterpart should be extracted
222+
addFile(true, LGTM_SRC, "normal.js");
223+
runTest();
224+
}
225+
226+
@Test
227+
public void skipFilesInTsconfigOutDir() throws IOException {
228+
envVars.put("LGTM_INDEX_TYPESCRIPT", "basic");
229+
// Files under outDir in tsconfig.json should be excluded
230+
// Create tsconfig.json with outDir set to "dist"
231+
addFile(true, LGTM_SRC, "tsconfig.json");
232+
Path config = Paths.get(LGTM_SRC.toString(), "tsconfig.json");
233+
Files.write(config,
234+
"{\"compilerOptions\":{\"outDir\":\"dist\"}}".getBytes(StandardCharsets.UTF_8));
235+
// Add files outside outDir (should be extracted)
236+
addFile(true, LGTM_SRC, "src", "app.ts");
237+
addFile(true, LGTM_SRC, "main.js");
238+
// Add files under dist/outDir (should be skipped)
239+
addFile(false, LGTM_SRC, "dist", "generated.js");
240+
addFile(false, LGTM_SRC, "dist", "sub", "x.js");
241+
runTest();
242+
}
243+
206244
@Test
207245
public void includeFile() throws IOException {
208246
envVars.put("LGTM_INDEX_INCLUDE", "tst.js");
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 JavaScript extractor now skips generated JavaScript files if the original TypeScript files are already present. It also skips any files in the output directory specified in the `compilerOptions` part of the `tsconfig.json` file.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Improved data flow tracking through middleware to handle default value and similar patterns.
5+
* Added `req._parsedUrl` as a remote input source.

javascript/ql/lib/semmle/javascript/Routing.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -925,7 +925,7 @@ module Routing {
925925
private DataFlow::Node getAnAccessPathRhs(Node base, int n, string path) {
926926
// Assigned in the body of a route handler function, which is a middleware
927927
exists(RouteHandler handler | base = handler |
928-
result = AccessPath::getAnAssignmentTo(handler.getParameter(n).ref(), path) and
928+
result = AccessPath::getAnAssignmentTo(handler.getParameter(n).ref(), path).getALocalSource() and
929929
(
930930
exists(handler.getAContinuationInvocation())
931931
or

javascript/ql/lib/semmle/javascript/frameworks/Express.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -618,9 +618,9 @@ module Express {
618618
kind = "body" and
619619
this = ref.getAPropertyRead("body")
620620
or
621-
// `req.path`
621+
// `req.path` and `req._parsedUrl`
622622
kind = "url" and
623-
this = ref.getAPropertyRead("path")
623+
this = ref.getAPropertyRead(["path", "_parsedUrl"])
624624
)
625625
}
626626

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@
66
| apollo.serverSide.ts:8:39:8:64 | get(fil ... => {}) | apollo.serverSide.ts:7:36:7:44 | { files } | apollo.serverSide.ts:8:43:8:50 | file.url | The $@ of this request depends on a $@. | apollo.serverSide.ts:8:43:8:50 | file.url | URL | apollo.serverSide.ts:7:36:7:44 | { files } | user-provided value |
77
| apollo.serverSide.ts:18:37:18:62 | get(fil ... => {}) | apollo.serverSide.ts:17:34:17:42 | { files } | apollo.serverSide.ts:18:41:18:48 | file.url | The $@ of this request depends on a $@. | apollo.serverSide.ts:18:41:18:48 | file.url | URL | apollo.serverSide.ts:17:34:17:42 | { files } | user-provided value |
88
| axiosInterceptors.serverSide.js:11:26:11:40 | userProvidedUrl | axiosInterceptors.serverSide.js:19:21:19:28 | req.body | axiosInterceptors.serverSide.js:11:26:11:40 | userProvidedUrl | The $@ of this request depends on a $@. | axiosInterceptors.serverSide.js:11:26:11:40 | userProvidedUrl | endpoint | axiosInterceptors.serverSide.js:19:21:19:28 | req.body | user-provided value |
9+
| serverSide2.js:17:28:17:47 | axios.get(targetUrl) | serverSide2.js:10:25:10:31 | req.url | serverSide2.js:17:38:17:46 | targetUrl | The $@ of this request depends on a $@. | serverSide2.js:17:38:17:46 | targetUrl | URL | serverSide2.js:10:25:10:31 | req.url | user-provided value |
10+
| serverSide2.js:20:29:20:49 | axios.g ... etUrl1) | serverSide2.js:9:43:9:56 | req._parsedUrl | serverSide2.js:20:39:20:48 | targetUrl1 | The $@ of this request depends on a $@. | serverSide2.js:20:39:20:48 | targetUrl1 | URL | serverSide2.js:9:43:9:56 | req._parsedUrl | user-provided value |
11+
| serverSide2.js:23:29:23:49 | axios.g ... etUrl2) | serverSide2.js:22:24:22:30 | req.url | serverSide2.js:23:39:23:48 | targetUrl2 | The $@ of this request depends on a $@. | serverSide2.js:23:39:23:48 | targetUrl2 | URL | serverSide2.js:22:24:22:30 | req.url | user-provided value |
12+
| serverSide2.js:26:29:26:49 | axios.g ... etUrl3) | serverSide2.js:11:24:11:30 | req.url | serverSide2.js:26:39:26:48 | targetUrl3 | The $@ of this request depends on a $@. | serverSide2.js:26:39:26:48 | targetUrl3 | URL | serverSide2.js:11:24:11:30 | req.url | user-provided value |
913
| serverSide.js:18:5:18:20 | request(tainted) | serverSide.js:14:29:14:35 | req.url | serverSide.js:18:13:18:19 | tainted | The $@ of this request depends on a $@. | serverSide.js:18:13:18:19 | tainted | URL | serverSide.js:14:29:14:35 | req.url | user-provided value |
1014
| serverSide.js:20:5:20:24 | request.get(tainted) | serverSide.js:14:29:14:35 | req.url | serverSide.js:20:17:20:23 | tainted | The $@ of this request depends on a $@. | serverSide.js:20:17:20:23 | tainted | URL | serverSide.js:14:29:14:35 | req.url | user-provided value |
1115
| serverSide.js:24:5:24:20 | request(options) | serverSide.js:14:29:14:35 | req.url | serverSide.js:23:19:23:25 | tainted | The $@ of this request depends on a $@. | serverSide.js:23:19:23:25 | tainted | URL | serverSide.js:14:29:14:35 | req.url | user-provided value |
@@ -63,6 +67,18 @@ edges
6367
| axiosInterceptors.serverSide.js:19:21:19:28 | req.body | axiosInterceptors.serverSide.js:19:11:19:17 | { url } | provenance | |
6468
| axiosInterceptors.serverSide.js:20:5:20:25 | userProvidedUrl | axiosInterceptors.serverSide.js:11:26:11:40 | userProvidedUrl | provenance | |
6569
| axiosInterceptors.serverSide.js:20:23:20:25 | url | axiosInterceptors.serverSide.js:20:5:20:25 | userProvidedUrl | provenance | |
70+
| serverSide2.js:9:34:9:63 | qs.pars ... .query) | serverSide2.js:19:24:19:51 | req.par ... rsedUrl | provenance | |
71+
| serverSide2.js:9:43:9:56 | req._parsedUrl | serverSide2.js:9:34:9:63 | qs.pars ... .query) | provenance | |
72+
| serverSide2.js:10:25:10:31 | req.url | serverSide2.js:16:23:16:41 | req.parsedQuery.url | provenance | |
73+
| serverSide2.js:11:24:11:30 | req.url | serverSide2.js:25:24:25:41 | req.SomeObject.url | provenance | |
74+
| serverSide2.js:16:11:16:41 | targetUrl | serverSide2.js:17:38:17:46 | targetUrl | provenance | |
75+
| serverSide2.js:16:23:16:41 | req.parsedQuery.url | serverSide2.js:16:11:16:41 | targetUrl | provenance | |
76+
| serverSide2.js:19:11:19:55 | targetUrl1 | serverSide2.js:20:39:20:48 | targetUrl1 | provenance | |
77+
| serverSide2.js:19:24:19:51 | req.par ... rsedUrl | serverSide2.js:19:11:19:55 | targetUrl1 | provenance | |
78+
| serverSide2.js:22:11:22:36 | targetUrl2 | serverSide2.js:23:39:23:48 | targetUrl2 | provenance | |
79+
| serverSide2.js:22:24:22:30 | req.url | serverSide2.js:22:11:22:36 | targetUrl2 | provenance | |
80+
| serverSide2.js:25:11:25:47 | targetUrl3 | serverSide2.js:26:39:26:48 | targetUrl3 | provenance | |
81+
| serverSide2.js:25:24:25:41 | req.SomeObject.url | serverSide2.js:25:11:25:47 | targetUrl3 | provenance | |
6682
| serverSide.js:14:9:14:52 | tainted | serverSide.js:18:13:18:19 | tainted | provenance | |
6783
| serverSide.js:14:9:14:52 | tainted | serverSide.js:20:17:20:23 | tainted | provenance | |
6884
| serverSide.js:14:9:14:52 | tainted | serverSide.js:23:19:23:25 | tainted | provenance | |
@@ -163,6 +179,22 @@ nodes
163179
| axiosInterceptors.serverSide.js:19:21:19:28 | req.body | semmle.label | req.body |
164180
| axiosInterceptors.serverSide.js:20:5:20:25 | userProvidedUrl | semmle.label | userProvidedUrl |
165181
| axiosInterceptors.serverSide.js:20:23:20:25 | url | semmle.label | url |
182+
| serverSide2.js:9:34:9:63 | qs.pars ... .query) | semmle.label | qs.pars ... .query) |
183+
| serverSide2.js:9:43:9:56 | req._parsedUrl | semmle.label | req._parsedUrl |
184+
| serverSide2.js:10:25:10:31 | req.url | semmle.label | req.url |
185+
| serverSide2.js:11:24:11:30 | req.url | semmle.label | req.url |
186+
| serverSide2.js:16:11:16:41 | targetUrl | semmle.label | targetUrl |
187+
| serverSide2.js:16:23:16:41 | req.parsedQuery.url | semmle.label | req.parsedQuery.url |
188+
| serverSide2.js:17:38:17:46 | targetUrl | semmle.label | targetUrl |
189+
| serverSide2.js:19:11:19:55 | targetUrl1 | semmle.label | targetUrl1 |
190+
| serverSide2.js:19:24:19:51 | req.par ... rsedUrl | semmle.label | req.par ... rsedUrl |
191+
| serverSide2.js:20:39:20:48 | targetUrl1 | semmle.label | targetUrl1 |
192+
| serverSide2.js:22:11:22:36 | targetUrl2 | semmle.label | targetUrl2 |
193+
| serverSide2.js:22:24:22:30 | req.url | semmle.label | req.url |
194+
| serverSide2.js:23:39:23:48 | targetUrl2 | semmle.label | targetUrl2 |
195+
| serverSide2.js:25:11:25:47 | targetUrl3 | semmle.label | targetUrl3 |
196+
| serverSide2.js:25:24:25:41 | req.SomeObject.url | semmle.label | req.SomeObject.url |
197+
| serverSide2.js:26:39:26:48 | targetUrl3 | semmle.label | targetUrl3 |
166198
| serverSide.js:14:9:14:52 | tainted | semmle.label | tainted |
167199
| serverSide.js:14:19:14:42 | url.par ... , true) | semmle.label | url.par ... , true) |
168200
| serverSide.js:14:29:14:35 | req.url | semmle.label | req.url |
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
const express = require('express');
2+
const axios = require('axios');
3+
const qs = require('qs');
4+
5+
const app = express();
6+
const PORT = 3000;
7+
8+
app.use((req, res, next) => {
9+
req.parsedQueryFromParsedUrl = qs.parse(req._parsedUrl.query); // $Source[js/request-forgery]
10+
req.parsedQuery.url = req.url || {}; // $Source[js/request-forgery]
11+
req.SomeObject.url = req.url; // $Source[js/request-forgery]
12+
next();
13+
});
14+
15+
app.get('/proxy', async (req, res) => {
16+
const targetUrl = req.parsedQuery.url;
17+
const response = await axios.get(targetUrl); // $Alert[js/request-forgery]
18+
19+
const targetUrl1 = req.parsedQueryFromParsedUrl.url;
20+
const response1 = await axios.get(targetUrl1); // $Alert[js/request-forgery]
21+
22+
const targetUrl2 = req.url || {}; // $Source[js/request-forgery]
23+
const response2 = await axios.get(targetUrl2); // $Alert[js/request-forgery]
24+
25+
const targetUrl3 = req.SomeObject.url || {};
26+
const response3 = await axios.get(targetUrl3); // $Alert[js/request-forgery]
27+
});

rust/extractor/src/config.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ pub struct Config {
6262
pub qltest: bool,
6363
pub qltest_cargo_check: bool,
6464
pub qltest_dependencies: Vec<String>,
65+
pub qltest_use_nightly: bool,
6566
pub sysroot: Option<PathBuf>,
6667
pub sysroot_src: Option<PathBuf>,
6768
pub rustc_src: Option<PathBuf>,

rust/extractor/src/main.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ impl<'a> Extractor<'a> {
103103
}
104104
}
105105
translator.emit_source_file(&ast);
106+
translator.emit_truncated_diagnostics_message();
106107
translator.trap.commit().unwrap_or_else(|err| {
107108
error!(
108109
"Failed to write trap file for: {}: {}",

rust/extractor/src/qltest.rs

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ use std::path::Path;
88
use std::process::Command;
99
use tracing::info;
1010

11+
const EDITION: &str = "2021";
12+
const NIGHTLY: &str = "nightly-2025-06-01";
13+
1114
fn dump_lib() -> anyhow::Result<()> {
1215
let path_iterator = glob("*.rs").context("globbing test sources")?;
1316
let paths = path_iterator
@@ -29,8 +32,11 @@ enum TestCargoManifest<'a> {
2932
uses_proc_macro: bool,
3033
uses_main: bool,
3134
dependencies: &'a [String],
35+
edition: &'a str,
36+
},
37+
Macro {
38+
edition: &'a str,
3239
},
33-
Macro {},
3440
}
3541

3642
impl TestCargoManifest<'_> {
@@ -56,16 +62,26 @@ fn dump_cargo_manifest(dependencies: &[String]) -> anyhow::Result<()> {
5662
uses_proc_macro,
5763
uses_main: fs::exists("main.rs").context("checking existence of main.rs")?,
5864
dependencies,
65+
edition: EDITION,
5966
};
6067
if uses_proc_macro {
6168
TestCargoManifest::Workspace {}.dump("")?;
6269
lib_manifest.dump(".lib")?;
63-
TestCargoManifest::Macro {}.dump(".proc_macro")
70+
TestCargoManifest::Macro { edition: EDITION }.dump(".proc_macro")
6471
} else {
6572
lib_manifest.dump("")
6673
}
6774
}
6875

76+
fn dump_nightly_toolchain() -> anyhow::Result<()> {
77+
fs::write(
78+
"rust-toolchain.toml",
79+
format!("[toolchain]\nchannel = \"{NIGHTLY}\"\n"),
80+
)
81+
.context("writing rust-toolchain.toml")?;
82+
Ok(())
83+
}
84+
6985
fn set_sources(config: &mut Config) -> anyhow::Result<()> {
7086
let path_iterator = glob("**/*.rs").context("globbing test sources")?;
7187
config.inputs = path_iterator
@@ -79,6 +95,9 @@ pub(crate) fn prepare(config: &mut Config) -> anyhow::Result<()> {
7995
dump_lib()?;
8096
set_sources(config)?;
8197
dump_cargo_manifest(&config.qltest_dependencies)?;
98+
if config.qltest_use_nightly {
99+
dump_nightly_toolchain()?;
100+
}
82101
if config.qltest_cargo_check {
83102
let status = Command::new("cargo")
84103
.env("RUSTFLAGS", "-Awarnings")

rust/extractor/src/qltest_cargo.mustache

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ members = [".lib", ".proc_macro"]
1111
[package]
1212
name = "test"
1313
version = "0.0.1"
14-
edition = "2021"
14+
edition = "{{ edition }}"
1515
[lib]
1616
path = "{{#uses_proc_macro}}../{{/uses_proc_macro}}lib.rs"
1717
{{#uses_main}}
@@ -32,7 +32,7 @@ proc_macro = { path = "../.proc_macro" }
3232
[package]
3333
name = "proc_macro"
3434
version = "0.0.1"
35-
edition = "2021"
35+
edition = "{{ edition }}"
3636
[lib]
3737
path = "../proc_macro.rs"
3838
proc_macro = true

0 commit comments

Comments
 (0)