fix(backend): validate the S3 endpoint host before the restore download

downloadFileFromS3() built an S3StorageAdapter and called .download()
directly, skipping the isHostAllowed() private-IP/DNS-rebinding guard
that testConnection() applies elsewhere — an admin with backup.restore
could point the configured S3 endpoint at an internal/metadata address
for unauthenticated egress via the server.
This commit is contained in:
Paul Nothaft
2026-09-10 20:35:16 +02:00
parent c4d89c9d64
commit 89c6a673bf
2 changed files with 126 additions and 0 deletions
@@ -0,0 +1,109 @@
/**
* Blind SSRF via the restore S3 download path (GHSA-vm2x-c628-3cx5).
*
* downloadFileFromS3() built a bare S3StorageAdapter and called .download()
* directly, never running the DNS-resolving isHostAllowed() guard that
* testConnection() applies elsewhere — so an admin with backup.restore could
* point the request-supplied S3 endpoint at an internal/metadata address for
* unauthenticated egress via the server. `s3Config` here is fully attacker
* controlled (POST /api/admin/restore/validate and /restore/start take it
* straight from the request body — see routes/adminRestore.js), unlike the
* scheduled-backup S3 endpoint, which is vetted at settings-save time.
*/
const path = require('path');
const fs = require('fs');
const os = require('os');
process.env.NODE_ENV = 'test';
process.env.TEST_DATABASE_PATH = path.join(
fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-restores3ssrf-')), 'db.sqlite',
);
process.env.JWT_SECRET = process.env.JWT_SECRET || 'restores3ssrf-test-secret';
jest.mock('dns', () => {
const actual = jest.requireActual('dns');
return { ...actual, promises: { ...actual.promises, lookup: jest.fn() } };
});
jest.mock('../../src/services/storage/s3Storage', () =>
jest.fn().mockImplementation(() => ({
download: jest.fn().mockResolvedValue(undefined),
}))
);
const dns = require('dns');
const lookup = dns.promises.lookup;
const S3StorageAdapter = require('../../src/services/storage/s3Storage');
const { RestoreService } = require('../../src/services/restoreService');
describe('downloadFileFromS3 SSRF guard (GHSA-vm2x-c628-3cx5)', () => {
let restoreService;
let originalNodeEnv;
beforeEach(() => {
restoreService = new RestoreService();
lookup.mockReset();
S3StorageAdapter.mockClear();
originalNodeEnv = process.env.NODE_ENV;
process.env.NODE_ENV = 'production';
});
afterEach(() => {
process.env.NODE_ENV = originalNodeEnv;
});
it('rejects an endpoint hostname that resolves to a private/internal address before any network call', async () => {
lookup.mockResolvedValue([{ address: '10.0.0.5', family: 4 }]);
await expect(
restoreService.downloadFileFromS3(
's3://backups/manifest.json',
'/tmp/whatever/manifest.json',
{ endpoint: 'evil-rebind.example.com', accessKeyId: 'k', secretAccessKey: 's' }
)
).rejects.toThrow(/private or internal network address/i);
expect(S3StorageAdapter).not.toHaveBeenCalled();
});
it('rejects an endpoint hostname that resolves to the cloud metadata address', async () => {
lookup.mockResolvedValue([{ address: '169.254.169.254', family: 4 }]);
await expect(
restoreService.downloadFileFromS3(
's3://backups/manifest.json',
'/tmp/whatever/manifest.json',
{ endpoint: 'metadata-rebind.example.com', accessKeyId: 'k', secretAccessKey: 's' }
)
).rejects.toThrow(/private or internal network address/i);
expect(S3StorageAdapter).not.toHaveBeenCalled();
});
it('allows a legitimate public S3 endpoint through to download()', async () => {
lookup.mockResolvedValue([{ address: '93.184.216.34', family: 4 }]);
await restoreService.downloadFileFromS3(
's3://backups/manifest.json',
'/tmp/whatever/manifest.json',
{ endpoint: 's3.example-cdn.com', accessKeyId: 'k', secretAccessKey: 's' }
);
expect(S3StorageAdapter).toHaveBeenCalledTimes(1);
});
it('does not require the guard outside production (dev MinIO stays usable), but still downloads', async () => {
process.env.NODE_ENV = 'development';
lookup.mockResolvedValue([{ address: '10.0.0.5', family: 4 }]); // would be rejected in prod
await restoreService.downloadFileFromS3(
's3://backups/manifest.json',
'/tmp/whatever/manifest.json',
{ endpoint: 'localhost:9000', accessKeyId: 'k', secretAccessKey: 's' }
);
expect(lookup).not.toHaveBeenCalled();
expect(S3StorageAdapter).toHaveBeenCalledTimes(1);
});
});
+17
View File
@@ -1628,6 +1628,23 @@ END $$;`
throw new Error('Invalid S3 URL format');
}
// SSRF guard: this method calls S3StorageAdapter.download() directly
// rather than going through testConnection(), so it must re-run the same
// DNS-resolving host check testConnection() applies — otherwise an
// admin-configured S3 endpoint could point at a private/internal or
// cloud-metadata address for unauthenticated egress via the server.
// Prod-only, matching S3StorageAdapter's own gate (dev points at
// localhost MinIO deliberately).
if (process.env.NODE_ENV === 'production' && s3Config && s3Config.endpoint) {
const { isHostAllowed } = require('../utils/networkValidation');
const { hostname } = new URL(
/^https?:\/\//.test(s3Config.endpoint) ? s3Config.endpoint : `https://${s3Config.endpoint}`
);
if (!(await isHostAllowed(hostname))) {
throw new Error('S3 endpoint resolves to a private or internal network address');
}
}
const [, bucket, key] = s3PathMatch;
const s3Client = new S3StorageAdapter({
...s3Config,