summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGravatar Erika <3019731+Princesseuh@users.noreply.github.com> 2024-03-01 10:23:07 +0100
committerGravatar GitHub <noreply@github.com> 2024-03-01 10:23:07 +0100
commit07f89429a1ef5173d3321e0b362a9dc71fc74fe5 (patch)
treeadac2bbe4bafa27fedbbb6f5eb94f83d55d08474
parentdf05138ebe498626ad97d005d9c634d2a9408d38 (diff)
downloadastro-07f89429a1ef5173d3321e0b362a9dc71fc74fe5.tar.gz
astro-07f89429a1ef5173d3321e0b362a9dc71fc74fe5.tar.zst
astro-07f89429a1ef5173d3321e0b362a9dc71fc74fe5.zip
fix(assets): Solidify Node endpoint (#10284)
* fix(assets): Solidify Node endpoint * chore: changeset
-rw-r--r--.changeset/soft-boxes-allow.md7
-rw-r--r--packages/astro/src/assets/endpoint/node.ts56
-rw-r--r--packages/astro/src/assets/vite-plugin-assets.ts3
-rw-r--r--packages/astro/test/core-image.test.js90
4 files changed, 143 insertions, 13 deletions
diff --git a/.changeset/soft-boxes-allow.md b/.changeset/soft-boxes-allow.md
new file mode 100644
index 000000000..54ff50ea8
--- /dev/null
+++ b/.changeset/soft-boxes-allow.md
@@ -0,0 +1,7 @@
+---
+"astro": patch
+---
+
+Fixes an issue where in Node SSR, the image endpoint could be used maliciously to reveal unintended information about the underlying system.
+
+Thanks to Google Security Team for reporting this issue.
diff --git a/packages/astro/src/assets/endpoint/node.ts b/packages/astro/src/assets/endpoint/node.ts
index cabf02a76..d06066fae 100644
--- a/packages/astro/src/assets/endpoint/node.ts
+++ b/packages/astro/src/assets/endpoint/node.ts
@@ -1,4 +1,7 @@
-import os from 'os';
+/* eslint-disable no-console */
+import os from 'node:os';
+import { isAbsolute } from 'node:path';
+import { fileURLToPath, pathToFileURL } from 'node:url';
import { isRemotePath, removeQueryString } from '@astrojs/internal-helpers/path';
import { readFile } from 'fs/promises';
import mime from 'mime/lite.js';
@@ -7,23 +10,44 @@ import { getConfiguredImageService } from '../internal.js';
import { etag } from '../utils/etag.js';
import { isRemoteAllowed } from '../utils/remotePattern.js';
// @ts-expect-error
-import { assetsDir, imageConfig } from 'astro:assets';
+import { assetsDir, outDir, imageConfig } from 'astro:assets';
function replaceFileSystemReferences(src: string) {
return os.platform().includes('win32') ? src.replace(/^\/@fs\//, '') : src.replace(/^\/@fs/, '');
}
async function loadLocalImage(src: string, url: URL) {
- const filePath = import.meta.env.DEV
- ? removeQueryString(replaceFileSystemReferences(src))
- : new URL('.' + src, assetsDir);
+ const assetsDirPath = fileURLToPath(assetsDir);
+
+ let fileUrl;
+ if (import.meta.env.DEV) {
+ fileUrl = pathToFileURL(removeQueryString(replaceFileSystemReferences(src)));
+ } else {
+ try {
+ fileUrl = new URL('.' + src, outDir);
+ const filePath = fileURLToPath(fileUrl);
+
+ if (!isAbsolute(filePath) || !filePath.startsWith(assetsDirPath)) {
+ return undefined;
+ }
+ } catch (err: unknown) {
+ return undefined;
+ }
+ }
+
let buffer: Buffer | undefined = undefined;
try {
- buffer = await readFile(filePath);
+ buffer = await readFile(fileUrl);
} catch (e) {
- const sourceUrl = new URL(src, url.origin);
- buffer = await loadRemoteImage(sourceUrl);
+ // Fallback to try to load the file using `fetch`
+ try {
+ const sourceUrl = new URL(src, url.origin);
+ buffer = await loadRemoteImage(sourceUrl);
+ } catch (err: unknown) {
+ console.error('Could not process image request:', err);
+ return undefined;
+ }
}
return buffer;
@@ -58,7 +82,11 @@ export const GET: APIRoute = async ({ request }) => {
const transform = await imageService.parseURL(url, imageConfig);
if (!transform?.src) {
- throw new Error('Incorrect transform returned by `parseURL`');
+ const err = new Error(
+ 'Incorrect transform returned by `parseURL`. Expected a transform with a `src` property.'
+ );
+ console.error('Could not parse image transform from URL:', err);
+ return new Response('Internal Server Error', { status: 500 });
}
let inputBuffer: Buffer | undefined = undefined;
@@ -74,7 +102,7 @@ export const GET: APIRoute = async ({ request }) => {
}
if (!inputBuffer) {
- return new Response('Not Found', { status: 404 });
+ return new Response('Internal Server Error', { status: 500 });
}
const { data, format } = await imageService.transform(inputBuffer, transform, imageConfig);
@@ -89,8 +117,12 @@ export const GET: APIRoute = async ({ request }) => {
},
});
} catch (err: unknown) {
- // eslint-disable-next-line no-console
console.error('Could not process image request:', err);
- return new Response(`Server Error: ${err}`, { status: 500 });
+ return new Response(
+ import.meta.env.DEV ? `Could not process image request: ${err}` : `Internal Server Error`,
+ {
+ status: 500,
+ }
+ );
}
};
diff --git a/packages/astro/src/assets/vite-plugin-assets.ts b/packages/astro/src/assets/vite-plugin-assets.ts
index 5a56e76a6..6afd01c2a 100644
--- a/packages/astro/src/assets/vite-plugin-assets.ts
+++ b/packages/astro/src/assets/vite-plugin-assets.ts
@@ -66,13 +66,14 @@ export default function assets({
export { default as Picture } from "astro/components/Picture.astro";
export const imageConfig = ${JSON.stringify(settings.config.image)};
- export const assetsDir = new URL(${JSON.stringify(
+ export const outDir = new URL(${JSON.stringify(
new URL(
isServerLikeOutput(settings.config)
? settings.config.build.client
: settings.config.outDir
)
)});
+ export const assetsDir = new URL(${JSON.stringify(settings.config.build.assets)}, outDir);
export const getImage = async (options) => await getImageInternal(options, imageConfig);
`;
}
diff --git a/packages/astro/test/core-image.test.js b/packages/astro/test/core-image.test.js
index 590e77000..3f53fba96 100644
--- a/packages/astro/test/core-image.test.js
+++ b/packages/astro/test/core-image.test.js
@@ -1103,6 +1103,96 @@ describe('astro:image', () => {
assert.equal(response.status, 200);
});
+ it('endpoint handle malformed requests', async () => {
+ const badPaths = [
+ '../../../../../../../../../../../../etc/hosts%00',
+ '../../../../../../../../../../../../etc/hosts',
+ '../../boot.ini',
+ '/../../../../../../../../%2A',
+ '../../../../../../../../../../../../etc/passwd%00',
+ '../../../../../../../../../../../../etc/passwd',
+ '../../../../../../../../../../../../etc/shadow%00',
+ '../../../../../../../../../../../../etc/shadow',
+ '/../../../../../../../../../../etc/passwd^^',
+ '/../../../../../../../../../../etc/shadow^^',
+ '/../../../../../../../../../../etc/passwd',
+ '/../../../../../../../../../../etc/shadow',
+ '/./././././././././././etc/passwd',
+ '/./././././././././././etc/shadow',
+ '....................etcpasswd',
+ '....................etcshadow',
+ '....................etcpasswd',
+ '....................etcshadow',
+ '/..../..../..../..../..../..../etc/passwd',
+ '/..../..../..../..../..../..../etc/shadow',
+ '.\\./.\\./.\\./.\\./.\\./.\\./etc/passwd',
+ '.\\./.\\./.\\./.\\./.\\./.\\./etc/shadow',
+ '....................etcpasswd%00',
+ '....................etcshadow%00',
+ '....................etcpasswd%00',
+ '....................etcshadow%00',
+ '%0a/bin/cat%20/etc/passwd',
+ '%0a/bin/cat%20/etc/shadow',
+ '%00/etc/passwd%00',
+ '%00/etc/shadow%00',
+ '%00../../../../../../etc/passwd',
+ '%00../../../../../../etc/shadow',
+ '/../../../../../../../../../../../etc/passwd%00.jpg',
+ '/../../../../../../../../../../../etc/passwd%00.html',
+ '/..%c0%af../..%c0%af../..%c0%af../..%c0%af../..%c0%af../..%c0%af../etc/passwd',
+ '/..%c0%af../..%c0%af../..%c0%af../..%c0%af../..%c0%af../..%c0%af../etc/shadow',
+ '/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/etc/passwd,',
+ '/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/etc/shadow,',
+ '%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%,25%5c..%25%5c..%25%5c..%25%5c..%00',
+ '/%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..,%25%5c..%25%5c..%25%5c..%25%5c..%00',
+ '%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%,25%5c..%25%5c..% 25%5c..%25%5c..%00',
+ '%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%,25%5c..%25%5c..% 25%5c..%25%5c..%255cboot.ini',
+ '/%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..%25%5c..,%25%5c..%25%5c..%25%5c..%25%5c..winnt/desktop.ini',
+ '\\&apos;/bin/cat%20/etc/passwd\\&apos;',
+ '\\&apos;/bin/cat%20/etc/shadow\\&apos;',
+ '../../../../../../../../conf/server.xml',
+ '/../../../../../../../../bin/id|',
+ 'C:/inetpub/wwwroot/global.asa',
+ 'C:inetpubwwwrootglobal.asa',
+ 'C:/boot.ini',
+ 'C:\boot.ini',
+ '../../../../../../../../../../../../localstart.asp%00',
+ '../../../../../../../../../../../../localstart.asp',
+ '../../../../../../../../../../../../boot.ini%00',
+ '../../../../../../../../../../../../boot.ini',
+ '/./././././././././././boot.ini',
+ '/../../../../../../../../../../../boot.ini%00',
+ '/../../../../../../../../../../../boot.ini',
+ '/..../..../..../..../..../..../boot.ini',
+ '/.\\./.\\./.\\./.\\./.\\./.\\./boot.ini',
+ '....................\boot.ini',
+ '....................\boot.ini%00',
+ '....................\boot.ini',
+ '/../../../../../../../../../../../boot.ini%00.html',
+ '/../../../../../../../../../../../boot.ini%00.jpg',
+ '/.../.../.../.../.../ ',
+ '..%c0%af../..%c0%af../..%c0%af../..%c0%af../..%c0%af../..%c0%af../boot.ini',
+ '/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/boot.ini',
+ '../prerender/index.html',
+ ];
+
+ const app = await fixture.loadTestAdapterApp();
+
+ for (const path of badPaths) {
+ let request = new Request('http://example.com/_image?href=' + path);
+ let response = await app.render(request);
+ const body = await response.text();
+
+ assert.equal(response.status, 500);
+ assert.equal(body.includes('Internal Server Error'), true);
+ }
+
+ // Server should still be running
+ let request = new Request('http://example.com/');
+ let response = await app.render(request);
+ assert.equal(response.status, 200);
+ });
+
it('prerendered routes images are built', async () => {
const html = await fixture.readFile('/client/prerender/index.html');
const $ = cheerio.load(html);