Skip to content

Commit

Permalink
fix(assets): Solidify Node endpoint (#10284)
Browse files Browse the repository at this point in the history 
* fix(assets): Solidify Node endpoint

* chore: changeset
  • Loading branch information
@Princesseuh
Princesseuh committed Mar 1, 2024
1 parent df05138 commit 07f8942
Show file tree
Hide file tree
Showing 4 changed files with 143 additions and 13 deletions.
7 changes: 7 additions & 0 deletions .changeset/soft-boxes-allow.md
View file
Original file line number Diff line number Diff line change
@@ -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.
56 changes: 44 additions & 12 deletions packages/astro/src/assets/endpoint/node.ts
View file
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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,
}
);
}
};
3 changes: 2 additions & 1 deletion packages/astro/src/assets/vite-plugin-assets.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
`;
}
Expand Down
90 changes: 90 additions & 0 deletions packages/astro/test/core-image.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 07f8942

Please sign in to comment.