Skip to content

Commit e430aa9

Browse files
authored
Merge pull request #20916 from asgerf/js/next-folders2
JS: Handle Next.js files named 'page' or 'route'
2 parents 40a9136 + d2e6ae5 commit e430aa9

File tree

6 files changed

+68
-20
lines changed

6 files changed

+68
-20
lines changed

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

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,39 +13,40 @@ module NextJS {
1313
*/
1414
PackageJson getANextPackage() { result.getDependencies().getADependency("next", _) }
1515

16-
bindingset[base, name]
17-
pragma[inline_late]
18-
private Folder getOptionalFolder(Folder base, string name) {
19-
result = base.getFolder(name)
20-
or
21-
not exists(base.getFolder(name)) and
22-
result = base
23-
}
24-
2516
private Folder packageRoot() { result = getANextPackage().getFile().getParentContainer() }
2617

27-
private Folder srcRoot() { result = getOptionalFolder(packageRoot(), "src") }
18+
private Folder srcRoot() { result = [packageRoot(), packageRoot().getFolder("src")] }
2819

2920
private Folder appRoot() { result = srcRoot().getFolder("app") }
3021

3122
private Folder pagesRoot() { result = [srcRoot(), appRoot()].getFolder("pages") }
3223

3324
private Folder apiRoot() { result = [pagesRoot(), appRoot()].getFolder("api") }
3425

26+
private Folder appFolder() {
27+
result = appRoot()
28+
or
29+
result = appFolder().getAFolder()
30+
}
31+
32+
private Folder pagesFolder() {
33+
result = pagesRoot()
34+
or
35+
result = pagesFolder().getAFolder()
36+
}
37+
3538
/**
3639
* Gets a "pages" folder in a `Next.js` application.
3740
* JavaScript files inside these folders are mapped to routes.
3841
*/
39-
Folder getAPagesFolder() {
40-
result = pagesRoot()
41-
or
42-
result = getAPagesFolder().getAFolder()
43-
}
42+
deprecated predicate getAPagesFolder = pagesFolder/0;
4443

4544
/**
46-
* Gets a module corrosponding to a `Next.js` page.
45+
* Gets a module corresponding to a `Next.js` page.
4746
*/
48-
Module getAPagesModule() { result.getFile().getParentContainer() = getAPagesFolder() }
47+
Module getAPagesModule() {
48+
result.getFile() = [pagesFolder().getAFile(), appFolder().getJavaScriptFile("page")]
49+
}
4950

5051
/**
5152
* Gets a module inside a "pages" folder where `fallback` from `getStaticPaths` is not set to false.
@@ -300,11 +301,17 @@ module NextJS {
300301
class NextAppRouteHandler extends DataFlow::FunctionNode, Http::Servers::StandardRouteHandler {
301302
NextAppRouteHandler() {
302303
exists(Module mod |
303-
mod.getFile().getParentContainer() = apiFolder() or
304-
mod.getFile().getStem() = "middleware"
304+
(
305+
mod.getFile().getParentContainer() = apiFolder()
306+
or
307+
mod.getFile().getStem() = "middleware"
308+
or
309+
mod.getFile().getStem() = "route" and mod.getFile().getParentContainer() = appFolder()
310+
)
305311
|
306312
this =
307-
mod.getAnExportedValue([any(Http::RequestMethodName m), "middleware"]).getAFunctionValue()
313+
mod.getAnExportedValue([any(Http::RequestMethodName m), "middleware", "proxy"])
314+
.getAFunctionValue()
308315
)
309316
}
310317

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Fixed a bug in the Next.js model that would cause the analysis to miss server-side taint sources in files
5+
named `route` or `page` appearing outside `api` and `pages` folders.

javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@
3535
| app/api/routeNextRequest.ts:15:20:15:23 | body | app/api/routeNextRequest.ts:4:22:4:31 | req.json() | app/api/routeNextRequest.ts:15:20:15:23 | body | Cross-site scripting vulnerability due to a $@. | app/api/routeNextRequest.ts:4:22:4:31 | req.json() | user-provided value |
3636
| app/api/routeNextRequest.ts:27:20:27:23 | body | app/api/routeNextRequest.ts:4:22:4:31 | req.json() | app/api/routeNextRequest.ts:27:20:27:23 | body | Cross-site scripting vulnerability due to a $@. | app/api/routeNextRequest.ts:4:22:4:31 | req.json() | user-provided value |
3737
| app/api/routeNextRequest.ts:31:27:31:30 | body | app/api/routeNextRequest.ts:4:22:4:31 | req.json() | app/api/routeNextRequest.ts:31:27:31:30 | body | Cross-site scripting vulnerability due to a $@. | app/api/routeNextRequest.ts:4:22:4:31 | req.json() | user-provided value |
38+
| app/blah/page.jsx:8:13:8:19 | req.url | app/blah/page.jsx:8:13:8:19 | req.url | app/blah/page.jsx:8:13:8:19 | req.url | Cross-site scripting vulnerability due to a $@. | app/blah/page.jsx:8:13:8:19 | req.url | user-provided value |
39+
| app/blah/page.jsx:15:13:15:19 | req.url | app/blah/page.jsx:15:13:15:19 | req.url | app/blah/page.jsx:15:13:15:19 | req.url | Cross-site scripting vulnerability due to a $@. | app/blah/page.jsx:15:13:15:19 | req.url | user-provided value |
40+
| app/blah/route.ts:3:25:3:27 | url | app/blah/route.ts:2:17:2:23 | req.url | app/blah/route.ts:3:25:3:27 | url | Cross-site scripting vulnerability due to a $@. | app/blah/route.ts:2:17:2:23 | req.url | user-provided value |
3841
| app/pages/Next2.jsx:8:13:8:19 | req.url | app/pages/Next2.jsx:8:13:8:19 | req.url | app/pages/Next2.jsx:8:13:8:19 | req.url | Cross-site scripting vulnerability due to a $@. | app/pages/Next2.jsx:8:13:8:19 | req.url | user-provided value |
3942
| app/pages/Next2.jsx:15:13:15:19 | req.url | app/pages/Next2.jsx:15:13:15:19 | req.url | app/pages/Next2.jsx:15:13:15:19 | req.url | Cross-site scripting vulnerability due to a $@. | app/pages/Next2.jsx:15:13:15:19 | req.url | user-provided value |
4043
| etherpad.js:11:12:11:19 | response | etherpad.js:9:16:9:30 | req.query.jsonp | etherpad.js:11:12:11:19 | response | Cross-site scripting vulnerability due to a $@. | etherpad.js:9:16:9:30 | req.query.jsonp | user-provided value |
@@ -149,6 +152,8 @@ edges
149152
| app/api/routeNextRequest.ts:4:9:4:12 | body | app/api/routeNextRequest.ts:31:27:31:30 | body | provenance | |
150153
| app/api/routeNextRequest.ts:4:16:4:31 | await req.json() | app/api/routeNextRequest.ts:4:9:4:12 | body | provenance | |
151154
| app/api/routeNextRequest.ts:4:22:4:31 | req.json() | app/api/routeNextRequest.ts:4:16:4:31 | await req.json() | provenance | |
155+
| app/blah/route.ts:2:11:2:13 | url | app/blah/route.ts:3:25:3:27 | url | provenance | |
156+
| app/blah/route.ts:2:17:2:23 | req.url | app/blah/route.ts:2:11:2:13 | url | provenance | |
152157
| etherpad.js:9:5:9:12 | response | etherpad.js:11:12:11:19 | response | provenance | |
153158
| etherpad.js:9:16:9:30 | req.query.jsonp | etherpad.js:9:5:9:12 | response | provenance | |
154159
| formatting.js:4:9:4:12 | evil | formatting.js:6:43:6:46 | evil | provenance | |
@@ -357,6 +362,11 @@ nodes
357362
| app/api/routeNextRequest.ts:15:20:15:23 | body | semmle.label | body |
358363
| app/api/routeNextRequest.ts:27:20:27:23 | body | semmle.label | body |
359364
| app/api/routeNextRequest.ts:31:27:31:30 | body | semmle.label | body |
365+
| app/blah/page.jsx:8:13:8:19 | req.url | semmle.label | req.url |
366+
| app/blah/page.jsx:15:13:15:19 | req.url | semmle.label | req.url |
367+
| app/blah/route.ts:2:11:2:13 | url | semmle.label | url |
368+
| app/blah/route.ts:2:17:2:23 | req.url | semmle.label | req.url |
369+
| app/blah/route.ts:3:25:3:27 | url | semmle.label | url |
360370
| app/pages/Next2.jsx:8:13:8:19 | req.url | semmle.label | req.url |
361371
| app/pages/Next2.jsx:15:13:15:19 | req.url | semmle.label | req.url |
362372
| etherpad.js:9:5:9:12 | response | semmle.label | response |

javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@
3434
| app/api/routeNextRequest.ts:15:20:15:23 | body | Cross-site scripting vulnerability due to $@. | app/api/routeNextRequest.ts:4:22:4:31 | req.json() | user-provided value |
3535
| app/api/routeNextRequest.ts:27:20:27:23 | body | Cross-site scripting vulnerability due to $@. | app/api/routeNextRequest.ts:4:22:4:31 | req.json() | user-provided value |
3636
| app/api/routeNextRequest.ts:31:27:31:30 | body | Cross-site scripting vulnerability due to $@. | app/api/routeNextRequest.ts:4:22:4:31 | req.json() | user-provided value |
37+
| app/blah/page.jsx:8:13:8:19 | req.url | Cross-site scripting vulnerability due to $@. | app/blah/page.jsx:8:13:8:19 | req.url | user-provided value |
38+
| app/blah/page.jsx:15:13:15:19 | req.url | Cross-site scripting vulnerability due to $@. | app/blah/page.jsx:15:13:15:19 | req.url | user-provided value |
39+
| app/blah/route.ts:3:25:3:27 | url | Cross-site scripting vulnerability due to $@. | app/blah/route.ts:2:17:2:23 | req.url | user-provided value |
3740
| app/pages/Next2.jsx:8:13:8:19 | req.url | Cross-site scripting vulnerability due to $@. | app/pages/Next2.jsx:8:13:8:19 | req.url | user-provided value |
3841
| app/pages/Next2.jsx:15:13:15:19 | req.url | Cross-site scripting vulnerability due to $@. | app/pages/Next2.jsx:15:13:15:19 | req.url | user-provided value |
3942
| formatting.js:6:14:6:47 | util.fo ... , evil) | Cross-site scripting vulnerability due to $@. | formatting.js:4:16:4:29 | req.query.evil | user-provided value |
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
export default function Page() {
2+
return <span />;
3+
}
4+
5+
Page.getInitialProps = async (ctx) => {
6+
const req = ctx.req;
7+
const res = ctx.res;
8+
res.end(req.url); // $ Alert
9+
return {}
10+
}
11+
12+
export async function getServerSideProps(ctx) {
13+
const req = ctx.req;
14+
const res = ctx.res;
15+
res.end(req.url); // $ Alert
16+
return {
17+
props: {}
18+
}
19+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
export async function GET(req: Request) {
2+
const url = req.url; // $ Source
3+
return new Response(url, { headers: { "Content-Type": "text/html" } }); // $ Alert
4+
}

0 commit comments

Comments
 (0)