diff options
author | 2024-03-01 10:23:07 +0100 | |
---|---|---|
committer | 2024-03-01 10:23:07 +0100 | |
commit | 07f89429a1ef5173d3321e0b362a9dc71fc74fe5 (patch) | |
tree | adac2bbe4bafa27fedbbb6f5eb94f83d55d08474 | |
parent | df05138ebe498626ad97d005d9c634d2a9408d38 (diff) | |
download | astro-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.md | 7 | ||||
-rw-r--r-- | packages/astro/src/assets/endpoint/node.ts | 56 | ||||
-rw-r--r-- | packages/astro/src/assets/vite-plugin-assets.ts | 3 | ||||
-rw-r--r-- | packages/astro/test/core-image.test.js | 90 |
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', + '\\'/bin/cat%20/etc/passwd\\'', + '\\'/bin/cat%20/etc/shadow\\'', + '../../../../../../../../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); |