fix Unsafe shell command constructed from library input shell-quote() OS Command Injection

[odaysec]

amplify-cli/packages/amplify-go-function-runtime-provider/src/localinvoke.ts

Line 73 in e936bfd

const lambdaExecutablePath = path.join(lambdaExecutableDir, MAIN_BINARY);

To fix the problem, we should avoid using execa.command with a dynamically constructed command string. Instead, we can use execa with an array of arguments to safely pass the executable and its arguments without shell interpretation. This approach ensures that the input is not interpreted by the shell, preventing command injection.

1. Replace the use of execa.command with execa and pass the executable and arguments as an array.
2. Ensure that the lambdaExecutablePath and lambdaExecutableDir are passed as separate arguments to avoid shell interpretation.

Dynamically constructing a shell command with inputs from exported functions may inadvertently change the meaning of the shell command. Clients using the exported function may use inputs containing characters that the shell interprets in a special way, for instance quotes and spaces. This can result in the shell command misbehaving, or even allowing a malicious user to execute arbitrary commands on the system.

POC

The following shows a dynamically constructed shell command that downloads a file from a remote URL.

var cp = require("child_process");

module.exports = function download(path, callback) {
  cp.exec("wget " + path, callback);
}

The shell command will, however, fail to work as intended if the input contains spaces or other special characters interpreted in a special way by the shell. Even worse, a client might pass in user-controlled data, not knowing that the input is interpreted as a shell command. This could allow a malicious user to provide the input http://redactedaws.org; cat /etc/passwd in order to execute the command cat /etc/passwd. To avoid such potentially catastrophic behaviors, provide the inputs from exported functions as an argument that does not get interpreted by a shell:

var cp = require("child_process");

module.exports = function download(path, callback) {
  cp.execFile("wget", [path], callback);
}

As another example, consider the following code which is similar to the preceding, but pipes the output of wget into wc -l to count the number of lines in the downloaded file.

var cp = require("child_process");

module.exports = function download(path, callback) {
  cp.exec("wget " + path + " | wc -l", callback);
};

In this case, using child_process.execFile is not an option because the shell is needed to interpret the pipe operator. Instead, you can use shell-quote to escape the input before embedding it in the command:

var cp = require("child_process");

module.exports = function download(path, callback) {
  cp.exec("wget " + shellQuote.quote([path]) + " | wc -l", callback);
};

References

Command Injection
shell-quote
CWE-78
CWE-88

Checklist

• PR description included
• yarn test passes
• Tests are changed or added
• Relevant documentation is changed or added (and PR referenced)
• New AWS SDK calls or CloudFormation actions have been added to relevant test and service IAM policies
• Pull request labels are added

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[sobolk]

@odaysec Thank you for the contribution.

Please keep in mind that lambdaExecutableDir input to execa is prepared by the CLI using Amplify project's path. Therefore chances of successful exploitation are none.
Also, the "amplify project path" -> CLI -> execa data flow happens on single developer machine.

The lambdaExecutablePath is constructed here

amplify-cli/packages/amplify-go-function-runtime-provider/src/localinvoke.ts

Lines 72 to 73 in c55512f

	const lambdaExecutableDir = path.join(request.srcRoot, BIN_LOCAL);
	const lambdaExecutablePath = path.join(lambdaExecutableDir, MAIN_BINARY);

from request.srcRoot.

The request.srcRoot is created here

amplify-cli/packages/amplify-category-function/src/index.ts

Lines 233 to 244 in c55512f

	const resourcePath = path.join(pathManager.getBackendDirPath(), categoryName, resourceName);
	const { pluginId, functionRuntime }: FunctionBreadcrumbs = context.amplify.readBreadcrumbs(categoryName, resourceName);
	const runtimeManager: FunctionRuntimeLifecycleManager = await context.amplify.loadRuntimePlugin(context, pluginId);
	return ({ event }) =>
	runtimeManager.invoke({
	handler,
	event: JSON.stringify(event),
	runtime: functionRuntime,
	srcRoot: resourcePath,
	envVars,
	});

.

Amplify CLI controls all inputs involved in lambdaExecutablePath computation. resourceName is validated against the project before invoking the function.