Skip to content

Commit

Permalink
Format error messages as text to avoid XSS vulnerabilities (#2161)
Browse files Browse the repository at this point in the history
Mark error messages from api.zupass.org as text to ensure any <script>
tags in them are not executed. Those could come directly from user
input.

This was reported by Andrew (a different one, not me) at Devcon. The
risk here is low since api.zupass.org is a different domain with no
sensitive data in cookies or local storage, but still good to clean up
the risk that users could be tricked into something dangerous.

Two test URLs which demonstrate script injection via URL parameters on
api.zupass.org:

```
https://api-staging.zupass.org/sync/v3/load?blobKey=%3Cscript%3Ealert(%22andrewwashere%22)%3C%2fscript%3E

https://api.zupass.org/generic-issuance/api/feed/19ffc405-742f-4485-a831-2ea7e987666d/zu-thailand%3Cscript%3Ealert(%60Cookies%3A%20%24%7Bdocument.cookie%7D%2C%20LocalStorage%3A%20%24%7BJSON.stringify(localStorage)%7D%60)%3C%2Fscript%3E
```

Here are versions of those URLs which demonstrate the issue on my local
test server:

```
http://localhost:3002/sync/v3/load?blobKey=%3Cscript%3Ealert(%22andrewwashere%22)%3C%2fscript%3E

http://localhost:3002/generic-issuance/api/feed/d16ddd24-0155-4244-8799-80f2c4afdc5f/0%3Cscript%3Ealert(%60Cookies%3A%20%24%7Bdocument.cookie%7D%2C%20LocalStorage%3A%20%24%7BJSON.stringify(localStorage)%7D%60)%3C%2Fscript%3E
```

Attn: @antimatter15 who submitted an initial version of this in #2159 .
  • Loading branch information
artwyman authored Nov 12, 2024
1 parent 2a2ffe1 commit b34222c
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 5 deletions.
2 changes: 1 addition & 1 deletion apps/passport-server/src/routing/pcdHttpError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export function respondWithError(
if (!e.message) {
res.sendStatus(e.code);
} else {
res.status(e.code).send(e.message);
res.status(e.code).type("text").send(e.message);
}
} else if (e instanceof PCDHTTPJSONError) {
res.status(e.code).json(e.json);
Expand Down
4 changes: 2 additions & 2 deletions apps/passport-server/src/routing/routes/accountRoutes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ export function initAccountRoutes(
if (result.success) {
res.status(200).json(result.value);
} else {
res.status(403).send(result.error);
res.status(403).type("text").send(result.error);
}
}
);
Expand All @@ -262,7 +262,7 @@ export function initAccountRoutes(
if (result.success) {
res.status(200).json(result.value);
} else {
res.status(403).send(result.error);
res.status(403).type("text").send(result.error);
}
}
);
Expand Down
3 changes: 3 additions & 0 deletions apps/passport-server/src/routing/routes/healthCheckRoutes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export function initHealthcheckRoutes(
async (req: Request, res: Response) => {
res
.status(200)
.type("text")
.send(`Zupass Server - Cluster Test OK! PID: ${process.pid}`);
}
);
Expand All @@ -46,6 +47,7 @@ export function initHealthcheckRoutes(
async (req: Request, res: Response) => {
res
.status(200)
.type("text")
.send(
`Zupass Server - Cluster Test OK! PID: ${
process.pid
Expand All @@ -62,6 +64,7 @@ export function initHealthcheckRoutes(
async (req: Request, res: Response) => {
res
.status(200)
.type("text")
.send(
`Zupass Server - Cluster Test OK! PID: ${
process.pid
Expand Down
9 changes: 8 additions & 1 deletion apps/passport-server/src/util/telegramWebApp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,14 @@ export const closeWebviewHtml = `
`;

export const errorHtmlWithDetails = (error: Error): string => {
const errorMessage = error ? error.message.replace(/\n/g, "<br>") : "";
// Reformat linebreaks in error messages as <br>. Also replace any <> to
// avoid including of HTML (accidental, or XSS risks).
const errorMessage = error
? error.message
.replace(/\n/g, "<br>")
.replace(/</g, "&lt;")
.replace(/>/g, "&gt;")
: "";
const errorStack =
error instanceof Error && error.stack
? error.stack.replace(/\n/g, "<br>")
Expand Down
2 changes: 1 addition & 1 deletion apps/zupoll-server/src/routing/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export async function startServer(
) => {
console.error(`[ERROR] ${req.method} ${req.url}`);
console.error(err.stack);
res.status(500).send(err.message);
res.status(500).type("text").send(err.message);
}
);

Expand Down

0 comments on commit b34222c

Please sign in to comment.