From d3bc298f8a95084af69a51f660c7b42b993ece43 Mon Sep 17 00:00:00 2001
From: Andree Klattenhoff <andree.klattenhoff@open-xchange.com>
Date: Mon, 29 Jan 2024 11:23:54 +0100
Subject: [PATCH] Fix: OXUIB-2698: SSRF/XSS: Prevent to cache resources from
external origins
---
integration/caching_test.js | 9 +++++++--
integration/config_test.js | 2 +-
integration/update-version_test.js | 4 ++--
spec/app_root_test.js | 2 +-
spec/file_caching_test.js | 7 ++++++-
spec/redirect_test.js | 2 +-
spec/salt_test.js | 2 +-
spec/server_test.js | 4 ++--
spec/util.js | 7 ++++++-
spec/version_mismatches_test.js | 2 +-
src/errors.js | 12 ++++++++++++
src/files.js | 8 ++++++--
src/routes/serve-files.js | 3 ++-
13 files changed, 48 insertions(+), 16 deletions(-)
diff --git a/integration/caching_test.js b/integration/caching_test.js
index d92df15..de4e7f0 100644
--- a/integration/caching_test.js
+++ b/integration/caching_test.js
@@ -30,7 +30,7 @@ describe('File caching service', function () {
let app, pubClient
beforeEach(async function () {
- await mockConfig({ baseUrls: ['http://ui-server/'] })
+ await mockConfig({ baseUrls: ['http://ui-server'] })
mockFetch({
'http://ui-server': {
'/manifest.json': generateSimpleViteManifest({
@@ -62,7 +62,7 @@ describe('File caching service', function () {
const version = response.headers.version
const { client } = await import('../src/redis.js')
- expect(await client.get(getRedisKey({ version, name: 'viteManifests' }))).to.equal('{"index.html":{"file":"index.html","meta":{"baseUrl":"http://ui-server/"}}}')
+ expect(await client.get(getRedisKey({ version, name: 'viteManifests' }))).to.equal('{"index.html":{"file":"index.html","meta":{"baseUrl":"http://ui-server"}}}')
const redisData = await client.getBuffer(getRedisKey({ version, name: 'oxManifests:body' }))
expect(zlib.brotliDecompressSync(redisData || '').toString()).to.equal('[]')
})
@@ -95,4 +95,9 @@ describe('File caching service', function () {
const response2 = await app.inject({ url: '/demo.js', headers: { version } })
expect(response2.statusCode).to.equal(200)
})
+
+ it('does not fetch from origins not defined in baseUrls', async function () {
+ const response = await app.inject({ url: '//t989be0.netlify.app/xss.html' })
+ expect(response.statusCode).to.equal(400)
+ })
})
diff --git a/integration/config_test.js b/integration/config_test.js
index e8f60fb..53ec8c1 100644
--- a/integration/config_test.js
+++ b/integration/config_test.js
@@ -32,7 +32,7 @@ describe('Configuration', function () {
beforeEach(async function () {
// need to set the redis-prefix. Otherwise, the bull workers will interfere
process.env.REDIS_PREFIX = Math.random().toString()
- await mockConfig(config = { baseUrls: ['http://ui-server/'] })
+ await mockConfig(config = { baseUrls: ['http://ui-server'] })
mockFetch({
'http://ui-server': {
'/manifest.json': generateSimpleViteManifest({
diff --git a/integration/update-version_test.js b/integration/update-version_test.js
index 457c3be..10e1f73 100644
--- a/integration/update-version_test.js
+++ b/integration/update-version_test.js
@@ -33,7 +33,7 @@ describe('Updates the version', function () {
beforeEach(async function () {
// need to set the redis-prefix. Otherwise, the bull workers will interfere
process.env.REDIS_PREFIX = Math.random().toString()
- await mockConfig({ baseUrls: ['http://ui-server/'] })
+ await mockConfig({ baseUrls: ['http://ui-server'] })
mockFetch({
'http://ui-server': {
'/manifest.json': generateSimpleViteManifest({
@@ -109,7 +109,7 @@ describe('Updates the version', function () {
td.reset()
// need to set the redis-prefix. Otherwise, the bull workers will interfere
process.env.REDIS_PREFIX = Math.random().toString()
- await mockConfig({ baseUrls: ['http://ui-server/'] })
+ await mockConfig({ baseUrls: ['http://ui-server'] })
mockFetch({
'http://ui-server': {
'/manifest.json': generateSimpleViteManifest({
diff --git a/spec/app_root_test.js b/spec/app_root_test.js
index 80bd741..fa10859 100644
--- a/spec/app_root_test.js
+++ b/spec/app_root_test.js
@@ -29,7 +29,7 @@ describe('With different app root', function () {
let app
beforeEach(async function () {
- await mockConfig({ baseUrls: ['http://ui-server/'] })
+ await mockConfig({ baseUrls: ['http://ui-server'] })
const { client } = await mockRedis()
mockFetch({
'http://ui-server': {
diff --git a/spec/file_caching_test.js b/spec/file_caching_test.js
index ec9668e..d2e6831 100644
--- a/spec/file_caching_test.js
+++ b/spec/file_caching_test.js
@@ -37,7 +37,7 @@ describe('File caching service', function () {
let redis
beforeEach(async function () {
- await mockConfig({ baseUrls: ['http://ui-server/'] })
+ await mockConfig({ baseUrls: ['http://ui-server'] })
redis = await mockRedis()
mockFetch({
'http://ui-server': {}
@@ -273,4 +273,9 @@ describe('File caching service', function () {
// check for files in redis
expect(await redis.client.getBuffer('ui-middleware:554855300:/file.svg:body')).to.deep.equal(response.rawPayload)
})
+
+ it('does not fetch from origins not defined in baseUrls', async function () {
+ const response = await app.inject({ url: '//t989be0.netlify.app/xss.html' })
+ expect(response.statusCode).to.equal(400)
+ })
})
diff --git a/spec/redirect_test.js b/spec/redirect_test.js
index 1a8ff31..c31ac11 100644
--- a/spec/redirect_test.js
+++ b/spec/redirect_test.js
@@ -29,7 +29,7 @@ describe('Redirects', function () {
let app
before(async function () {
- await mockConfig({ baseUrls: ['http://ui-server/'] })
+ await mockConfig({ baseUrls: ['http://ui-server'] })
await mockRedis()
mockFetch({
'http://ui-server': {
diff --git a/spec/salt_test.js b/spec/salt_test.js
index 4a755b2..e3005cc 100644
--- a/spec/salt_test.js
+++ b/spec/salt_test.js
@@ -30,7 +30,7 @@ describe('Salt', function () {
let config
beforeEach(async function () {
- await mockConfig(config = { baseUrls: ['http://ui-server/'] })
+ await mockConfig(config = { baseUrls: ['http://ui-server'] })
await mockRedis()
mockFetch({
'http://ui-server': {
diff --git a/spec/server_test.js b/spec/server_test.js
index c35ad5c..481de17 100644
--- a/spec/server_test.js
+++ b/spec/server_test.js
@@ -30,7 +30,7 @@ describe('UI Middleware', function () {
let fetchConfig
beforeEach(async function () {
- await mockConfig({ baseUrls: ['http://ui-server/'] })
+ await mockConfig({ baseUrls: ['http://ui-server'] })
await mockRedis()
mockFetch(fetchConfig = {
'http://ui-server': {
@@ -77,7 +77,7 @@ describe('UI Middleware', function () {
describe('multiple configurations', function () {
beforeEach(async function () {
- await mockConfig({ baseUrls: ['http://ui-server/', 'http://ui-server2/'] })
+ await mockConfig({ baseUrls: ['http://ui-server', 'http://ui-server2'] })
fetchConfig['http://ui-server2'] = {
'/manifest.json': generateSimpleViteManifest({ 'example2.js': 'thing' }),
'/example2.js': ''
diff --git a/spec/util.js b/spec/util.js
index 4a67a48..2efef21 100644
--- a/spec/util.js
+++ b/spec/util.js
@@ -60,7 +60,12 @@ export function mockConfig (obj = {}) {
export function mockFetch (servers = {}) {
td.replace(global, 'fetch', async function ({ origin, pathname }, ...args) {
const response = servers[origin]?.[pathname]
- if (response === undefined) return new Response('', { status: 404 })
+ if (response === undefined) {
+ if (origin === 'http://t989be0.netlify.app') {
+ return new Response('<html><code>Proof of Concept</code><script>alert(document.domain)</script></html>', { status: 200, headers: { 'Content-Type': 'text/html' } })
+ }
+ return new Response('', { status: 404 })
+ }
if (response instanceof Function) return response.apply(this, arguments)
if (typeof response === 'object') {
diff --git a/spec/version_mismatches_test.js b/spec/version_mismatches_test.js
index 869a8ac..fc4e50b 100644
--- a/spec/version_mismatches_test.js
+++ b/spec/version_mismatches_test.js
@@ -32,7 +32,7 @@ describe('version mismatches', function () {
let runUpdate
beforeEach(async function () {
- await mockConfig({ baseUrls: ['http://ui-server/'] })
+ await mockConfig({ baseUrls: ['http://ui-server'] })
const { createClient } = await mockRedis()
mockFetch({
'http://ui-server': {
diff --git a/src/errors.js b/src/errors.js
index aa6157a..220e071 100644
--- a/src/errors.js
+++ b/src/errors.js
@@ -27,6 +27,13 @@ export class NotFoundError extends Error {
}
}
+export class NotAllowedOriginError extends Error {
+ constructor (message, options = {}) {
+ super(message, options)
+ this.status = options.status
+ }
+}
+
export class VersionMismatchError extends Error {}
/**
@@ -48,3 +55,8 @@ export function isNotFoundError (err) {
const errors = err instanceof AggregateError ? err.errors : [err]
return errors.some(error => error instanceof NotFoundError)
}
+
+export function isNotAllowedOriginError (err) {
+ const errors = err instanceof AggregateError ? err.errors : [err]
+ return errors.some(error => error instanceof NotAllowedOriginError)
+}
diff --git a/src/files.js b/src/files.js
index 6ce97ed..9137089 100644
--- a/src/files.js
+++ b/src/files.js
@@ -25,7 +25,7 @@ import { promisify } from 'node:util'
import zlib from 'node:zlib'
import * as cache from './cache.js'
import { configMap } from './config_map.js'
-import { NotFoundError, VersionMismatchError, isVersionMismatchError } from './errors.js'
+import { NotAllowedOriginError, NotFoundError, VersionMismatchError, isVersionMismatchError } from './errors.js'
import logger from './logger.js'
import { getCSSDependenciesFor, getViteManifests } from './manifests.js'
import { getVersionInfo } from './version.js'
@@ -38,8 +38,12 @@ const compressionMimeTypes = (process.env.COMPRESS_FILE_TYPES || '').replace(/([
const compressionWhitelistRegex = new RegExp(`^(${compressionMimeTypes.join('|')})($|;)`, 'i')
export async function fetchFileWithHeadersFromBaseUrl ({ path, baseUrl, version }) {
+ const upstreamUrl = new URL(path, baseUrl)
+ if (upstreamUrl.origin !== baseUrl) {
+ throw new NotAllowedOriginError('This origin is not allowed', { status: 400 })
+ }
const [response, dependencies] = await Promise.all([
- fetch(new URL(path, baseUrl), { cache: 'no-store' }),
+ fetch(upstreamUrl, { cache: 'no-store' }),
nodePath.extname(path) === '.js' && getCSSDependenciesFor({ file: path.substr(1), version })
])
diff --git a/src/routes/serve-files.js b/src/routes/serve-files.js
index 5455157..91faa7d 100644
--- a/src/routes/serve-files.js
+++ b/src/routes/serve-files.js
@@ -21,7 +21,7 @@
*/
import { getFile } from '../files.js'
-import { isNotFoundError, isVersionMismatchError } from '../errors.js'
+import { isNotAllowedOriginError, isNotFoundError, isVersionMismatchError } from '../errors.js'
export default async function serveFilePlugin (fastify, options) {
fastify.get('*', async (req, reply) => {
@@ -35,6 +35,7 @@ export default async function serveFilePlugin (fastify, options) {
reply.send(body)
} catch (err) {
if (isNotFoundError(err) || isVersionMismatchError(err)) throw fastify.httpErrors.createError(404, `File "${req.urlData('path')}" does not exist.`, err)
+ if (isNotAllowedOriginError(err)) throw fastify.httpErrors.createError(400, err)
throw fastify.httpErrors.createError(err.statusCode || 500, err)
}
})
--
GitLab