paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 40 Packages Projects Releases Wiki Activity

Merge pull request from GHSA-pwhf-39xg-4rxw

Browse Source 
Fix Advisory 1
This commit is contained in:
Patrik Oldsberg
2021-06-03 10:09:06 +02:00
committed by GitHub
parent 644d315080 2d1bc6e6d4
commit dae35dc48f
14 changed files with 267 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/techdocs-fresh-and-clean.md
+6
View File
@@ -0,0 +1,6 @@
---
'@backstage/techdocs-common': patch
'@backstage/plugin-techdocs': patch
---
Fixes multiple XSS and sanitization bypass vulnerabilities in TechDocs.
.github/styles/vocab.txt
+1
View File
@@ -210,6 +210,7 @@ rst
rsync
ruleset
sam
sanitization
scaffolded
scaffolder
Scaffolder
packages/techdocs-common/__mocks__/@google-cloud/storage.ts
+3
View File
@@ -63,6 +63,9 @@ class GCSFile {
process.nextTick(() => {
if (fs.existsSync(this.localFilePath)) {
if (readable.eventNames().includes('pipe')) {
readable.emit('pipe');
}
readable.emit('data', fs.readFileSync(this.localFilePath));
readable.emit('end');
} else {
packages/techdocs-common/src/stages/publish/awsS3.test.ts
+27
View File
@@ -286,8 +286,12 @@ describe('AwsS3Publish', () => {
mockFs.restore();
mockFs({
[entityRootDir]: {
html: {
'unsafe.html': '<html></html>',
},
img: {
'with spaces.png': 'found it',
'unsafe.svg': '<svg></svg>',
},
'some folder': {
'also with spaces.js': 'found it too',
@@ -318,5 +322,28 @@ describe('AwsS3Publish', () => {
);
expect(jsResponse.text).toEqual('found it too');
});
it('should pass text/plain content-type for html', async () => {
const {
kind,
metadata: { namespace, name },
} = entity;
const htmlResponse = await request(app).get(
`/${namespace}/${kind}/${name}/html/unsafe.html`,
);
expect(htmlResponse.text).toEqual('<html></html>');
expect(htmlResponse.header).toMatchObject({
'content-type': 'text/plain; charset=utf-8',
});
const svgResponse = await request(app).get(
`/${namespace}/${kind}/${name}/img/unsafe.svg`,
);
expect(svgResponse.text).toEqual('<svg></svg>');
expect(svgResponse.header).toMatchObject({
'content-type': 'text/plain; charset=utf-8',
});
});
});
});
packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts
+27
View File
@@ -343,8 +343,12 @@ describe('publishing with valid credentials', () => {
mockFs.restore();
mockFs({
[entityRootDir]: {
html: {
'unsafe.html': '<html></html>',
},
img: {
'with spaces.png': 'found it',
'unsafe.svg': '<svg></svg>',
},
'some folder': {
'also with spaces.js': 'found it too',
@@ -375,5 +379,28 @@ describe('publishing with valid credentials', () => {
);
expect(jsResponse.text).toEqual('found it too');
});
it('should pass text/plain content-type for html', async () => {
const {
kind,
metadata: { namespace, name },
} = entity;
const htmlResponse = await request(app).get(
`/${namespace}/${kind}/${name}/html/unsafe.html`,
);
expect(htmlResponse.text).toEqual('<html></html>');
expect(htmlResponse.header).toMatchObject({
'content-type': 'text/plain; charset=utf-8',
});
const svgResponse = await request(app).get(
`/${namespace}/${kind}/${name}/img/unsafe.svg`,
);
expect(svgResponse.text).toEqual('<svg></svg>');
expect(svgResponse.header).toMatchObject({
'content-type': 'text/plain; charset=utf-8',
});
});
});
});
packages/techdocs-common/src/stages/publish/googleStorage.test.ts
+30 -1
View File
@@ -285,8 +285,12 @@ describe('GoogleGCSPublish', () => {
mockFs.restore();
mockFs({
[entityRootDir]: {
html: {
'unsafe.html': '<html></html>',
},
img: {
'with spaces.png': 'found it',
'unsafe.svg': '<svg></svg>',
},
'some folder': {
'also with spaces.js': 'found it too',
@@ -309,11 +313,36 @@ describe('GoogleGCSPublish', () => {
const pngResponse = await request(app).get(
`/${namespace}/${kind}/${name}/img/with%20spaces.png`,
);
expect(pngResponse.text).toEqual('found it');
expect(Buffer.from(pngResponse.body).toString('utf8')).toEqual(
'found it',
);
const jsResponse = await request(app).get(
`/${namespace}/${kind}/${name}/some%20folder/also%20with%20spaces.js`,
);
expect(jsResponse.text).toEqual('found it too');
});
it('should pass text/plain content-type for html', async () => {
const {
kind,
metadata: { namespace, name },
} = entity;
const htmlResponse = await request(app).get(
`/${namespace}/${kind}/${name}/html/unsafe.html`,
);
expect(htmlResponse.text).toEqual('<html></html>');
expect(htmlResponse.header).toMatchObject({
'content-type': 'text/plain; charset=utf-8',
});
const svgResponse = await request(app).get(
`/${namespace}/${kind}/${name}/img/unsafe.svg`,
);
expect(svgResponse.text).toEqual('<svg></svg>');
expect(svgResponse.header).toMatchObject({
'content-type': 'text/plain; charset=utf-8',
});
});
});
});
packages/techdocs-common/src/stages/publish/helpers.test.ts
+8 -2
View File
@@ -20,12 +20,18 @@ import { getFileTreeRecursively, getHeadersForFileExtension } from './helpers';
describe('getHeadersForFileExtension', () => {
const correctMapOfExtensions = [
['.html', 'text/html; charset=utf-8'],
['.html', 'text/plain; charset=utf-8'],
['.htm', 'text/plain; charset=utf-8'],
['.HTML', 'text/plain; charset=utf-8'],
['.dhtml', 'text/plain; charset=utf-8'],
['.xhtml', 'text/plain; charset=utf-8'],
['.xml', 'text/plain; charset=utf-8'],
['.css', 'text/css; charset=utf-8'],
['.png', 'image/png'],
['.jpg', 'image/jpeg'],
['.jpeg', 'image/jpeg'],
['.svg', 'image/svg+xml'],
['.svg', 'text/plain; charset=utf-8'],
['.SVG', 'text/plain; charset=utf-8'],
['.json', 'application/json; charset=utf-8'],
['.this-in-not-an-extension', 'text/plain; charset=utf-8'],
];
packages/techdocs-common/src/stages/publish/helpers.ts
+18 -3
View File
@@ -16,6 +16,22 @@
import mime from 'mime-types';
import recursiveReadDir from 'recursive-readdir';
/**
* Helper to get the expected content-type for a given file extension. Also
* takes XSS mitigation into account.
*/
const getContentTypeForExtension = (ext: string): string => {
const defaultContentType = 'text/plain; charset=utf-8';
// Prevent sanitization bypass by preventing browsers from directly rendering
// the contents of untrusted files.
if (ext.match(/htm|xml|svg/i)) {
return defaultContentType;
}
return mime.contentType(ext) || defaultContentType;
};
export type responseHeadersType = {
'Content-Type': string;
};
@@ -29,9 +45,8 @@ export const getHeadersForFileExtension = (
fileExtension: string,
): responseHeadersType => {
return {
'Content-Type':
mime.contentType(fileExtension) || 'text/plain; charset=utf-8',
} as responseHeadersType;
'Content-Type': getContentTypeForExtension(fileExtension),
};
};
/**
packages/techdocs-common/src/stages/publish/local.test.ts
+49 -7
View File
@@ -16,8 +16,11 @@
import {
getVoidLogger,
PluginEndpointDiscovery,
resolvePackagePath,
} from '@backstage/backend-common';
import { ConfigReader } from '@backstage/config';
import express from 'express';
import request from 'supertest';
import mockFs from 'mock-fs';
import * as os from 'os';
import { LocalPublish } from './local';
@@ -35,11 +38,21 @@ const createMockEntity = (annotations = {}) => {
};
};
const testDiscovery: jest.Mocked<PluginEndpointDiscovery> = {
getBaseUrl: jest.fn().mockResolvedValue('http://localhost:7000/api/techdocs'),
getExternalBaseUrl: jest.fn(),
};
const logger = getVoidLogger();
const tmpDir =
os.platform() === 'win32' ? 'C:\\tmp\\generatedDir' : '/tmp/generatedDir';
const resolvedDir = resolvePackagePath(
'@backstage/plugin-techdocs-backend',
'static/docs',
);
describe('local publisher', () => {
it('should publish generated documentation dir', async () => {
mockFs({
@@ -48,13 +61,6 @@ describe('local publisher', () => {
},
});
const testDiscovery: jest.Mocked<PluginEndpointDiscovery> = {
getBaseUrl: jest
.fn()
.mockResolvedValue('http://localhost:7000/api/techdocs'),
getExternalBaseUrl: jest.fn(),
};
const mockConfig = new ConfigReader({});
const publisher = new LocalPublish(mockConfig, logger, testDiscovery);
@@ -66,4 +72,40 @@ describe('local publisher', () => {
mockFs.restore();
});
describe('docsRouter', () => {
const mockConfig = new ConfigReader({});
const publisher = new LocalPublish(mockConfig, logger, testDiscovery);
let app: express.Express;
beforeEach(() => {
app = express().use(publisher.docsRouter());
mockFs.restore();
mockFs({
[resolvedDir]: {
'unsafe.html': '<html></html>',
'unsafe.svg': '<svg></svg>',
},
});
});
afterEach(() => {
mockFs.restore();
});
it('should pass text/plain content-type for unsafe types', async () => {
const htmlResponse = await request(app).get(`/unsafe.html`);
expect(htmlResponse.text).toEqual('<html></html>');
expect(htmlResponse.header).toMatchObject({
'content-type': 'text/plain; charset=utf-8',
});
const svgResponse = await request(app).get(`/unsafe.svg`);
expect(svgResponse.text).toEqual('<svg></svg>');
expect(svgResponse.header).toMatchObject({
'content-type': 'text/plain; charset=utf-8',
});
});
});
});
packages/techdocs-common/src/stages/publish/local.ts
+11 -1
View File
@@ -31,6 +31,7 @@ import {
ReadinessResponse,
TechDocsMetadata,
} from './types';
import { getHeadersForFileExtension } from './helpers';
// TODO: Use a more persistent storage than node_modules or /tmp directory.
// Make it configurable with techdocs.publisher.local.publishDirectory
@@ -132,7 +133,16 @@ export class LocalPublish implements PublisherBase {
}
docsRouter(): express.Handler {
return express.static(staticDocsDir);
return express.static(staticDocsDir, {
// Handle content-type header the same as all other publishers.
setHeaders: (res, filePath) => {
const fileExtension = path.extname(filePath);
const headers = getHeadersForFileExtension(fileExtension);
for (const [header, value] of Object.entries(headers)) {
res.setHeader(header, value);
}
},
});
}
async hasDocsBeenGenerated(entity: Entity): Promise<boolean> {
packages/techdocs-common/src/stages/publish/openStackSwift.test.ts
+27
View File
@@ -291,7 +291,11 @@ describe('OpenStackSwiftPublish', () => {
mockFs.restore();
mockFs({
[entityRootDir]: {
html: {
'unsafe.html': '<html></html>',
},
img: {
'unsafe.svg': '<svg></svg>',
'with spaces.png': 'found it',
},
'some folder': {
@@ -323,5 +327,28 @@ describe('OpenStackSwiftPublish', () => {
);
expect(jsResponse.text).toEqual('found it too');
});
it('should pass text/plain content-type for unsafe types', async () => {
const {
kind,
metadata: { namespace, name },
} = entity;
const htmlResponse = await request(app).get(
`/${namespace}/${kind}/${name}/html/unsafe.html`,
);
expect(htmlResponse.text).toEqual('<html></html>');
expect(htmlResponse.header).toMatchObject({
'content-type': 'text/plain; charset=utf-8',
});
const svgResponse = await request(app).get(
`/${namespace}/${kind}/${name}/img/unsafe.svg`,
);
expect(svgResponse.text).toEqual('<svg></svg>');
expect(svgResponse.header).toMatchObject({
'content-type': 'text/plain; charset=utf-8',
});
});
});
});
plugins/techdocs/src/reader/transformers/addBaseUrl.test.ts
+40 -1
View File
@@ -47,8 +47,17 @@ const mockEntityId = {
namespace: '',
name: '',
};
describe('addBaseUrl', () => {
const originalFetch = global.fetch;
beforeEach(() => {
global.fetch = jest.fn();
});
afterEach(() => {
global.fetch = originalFetch;
});
it('contains relative paths', () => {
createTestShadowDom(fixture, {
preTransformers: [
@@ -86,4 +95,34 @@ describe('addBaseUrl', () => {
'',
);
});
it('transforms svg img src to data uri', async () => {
const svgContent = '<svg></svg>';
const expectedSrc = `data:image/svg+xml;base64,${Buffer.from(
svgContent,
).toString('base64')}`;
(global.fetch as jest.Mock).mockReturnValue({
text: jest.fn().mockResolvedValue(svgContent),
});
const root = createTestShadowDom('<img id="x" src="test.svg" />', {
preTransformers: [
addBaseUrl({
techdocsStorageApi,
entityId: mockEntityId,
path: '',
}),
],
postTransformers: [],
});
await new Promise<void>(done => {
process.nextTick(() => {
const actualSrc = root.getElementById('x')?.getAttribute('src');
expect(expectedSrc).toEqual(actualSrc);
done();
});
});
});
});
plugins/techdocs/src/reader/transformers/addBaseUrl.ts
+20 -3
View File
@@ -38,10 +38,27 @@ export const addBaseUrl = ({
.forEach(async (elem: T) => {
const elemAttribute = elem.getAttribute(attributeName);
if (!elemAttribute) return;
elem.setAttribute(
attributeName,
await techdocsStorageApi.getBaseUrl(elemAttribute, entityId, path),
// Special handling for SVG images.
const newValue = await techdocsStorageApi.getBaseUrl(
elemAttribute,
entityId,
path,
);
if (attributeName === 'src' && elemAttribute.endsWith('.svg')) {
try {
const svg = await fetch(newValue);
const svgContent = await svg.text();
elem.setAttribute(
attributeName,
`data:image/svg+xml;base64,${btoa(svgContent)}`,
);
} catch (e) {
elem.setAttribute('alt', `Error: ${elemAttribute}`);
}
} else {
elem.setAttribute(attributeName, newValue);
}
});
};
plugins/techdocs/src/reader/transformers/sanitizeDOM/tags.ts
-1
View File
@@ -153,7 +153,6 @@ export const svg = [
'mask',
'metadata',
'mpath',
'object',
'path',
'pattern',
'polygon',