Skip to content

Commit

Permalink
fix(security): require screenshot protocol to be http/https
Browse files Browse the repository at this point in the history
prevent file:// URI scheme in Playwright screenshots

A critical vulnerability was discovered in a web application feature that utilizes
Playwright's screenshot capability. Attackers could exploit this vulnerability by
using the file:// URI scheme to read arbitrary files on the server's filesystem,
potentially exposing sensitive information, such as AWS credentials.

This commit addresses the vulnerability by implementing proper input validation
and sanitization to prevent the use of the file:// URI scheme in Playwright
screenshot requests, mitigating the risk of unauthorized file access.

resolves #47
  • Loading branch information
jasonraimondi committed Jun 4, 2024
1 parent 18e9c30 commit 9336020
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 3 deletions.
19 changes: 17 additions & 2 deletions src/lib/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,25 @@ const zodStringBool = z
.transform(x => x === "true")
.pipe(z.boolean());

const zodStringUrl = z.string().url();
const urlSchema = z
.string()
.url()
.refine(
val => {
try {
const url = new URL(val);
return url.protocol === "http:" || url.protocol === "https:";
} catch (err) {
return false;
}
},
{
message: "must start with http or https",
},
);

export const PlainConfigSchema = z.object({
url: zodStringUrl,
url: urlSchema,
width: z.coerce.number().nullish(),
height: z.coerce.number().nullish(),
viewPortWidth: z.coerce.number().nullish(),
Expand Down
10 changes: 9 additions & 1 deletion src/middlewares/extract_query_params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,15 @@ export function handleExtractQueryParamsMiddleware(encryptionService?: StringEnc
const { validData, errors } = parseForm({ data: input, schema: PlainConfigSchema });

if (errors) {
throw new HTTPException(400, { message: "Invalid query parameters", cause: errors });
let message: string = "Invalid query parameters: ";

const specificErrors = Object.entries(errors).map(([key, value]) => `(${key} - ${value})`).join(" ")

message = `${message} ${specificErrors}`;

console.log(message);

throw new HTTPException(400, { message, cause: errors });
}

if (validData.width && validData.width > 1920) {
Expand Down
12 changes: 12 additions & 0 deletions tests/app.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,18 @@ suite("app", () => {
expect(res.status).toBe(400);
expect(await res.text()).toMatch(/Invalid query/gi);
});

[
"file:///etc/passwd&width=4000",
"view-source:file:///home/&width=4000",
"view-source:file:///home/ec2-user/url-to-png/.env",
].forEach(invalidDomain => {
it(`throws when invalid protocol ${invalidDomain}`, async () => {
const res = await app.request(`/?url=${invalidDomain}`);
expect(res.status).toBe(400);
expect(await res.text()).toMatch(/url - must start with http or https/gi);
});
});
});

describe("GET /?hash=", () => {
Expand Down

0 comments on commit 9336020

Please # to comment.