-
Notifications
You must be signed in to change notification settings - Fork 733
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Exec function calls JSON.stringify on command #772
Comments
I think your problem may not be with exec('foo --id 909845429478596609') ends up in the temporary JS file as: var childProcess = child.exec("foo --id 909845429478596609", {}, function(err) {
// ...
}); In other words, the individual parts of the command are not stringified, the whole command is. To illustrate how this prevents escaping and code-injection issues, take for example this command: exec('echo"); console.log("vulnerable"); console.log("') which ends up as: var childProcess = child.exec("echo\"); console.log(\"vulnerable\"); console.log(\"", {}, function(err) {
// ...
}); Without var childProcess = child.exec("echo"); console.log("vulnerable"); console.log("", {}, function(err) {
// ...
}); Anyways, this was a long way of saying that the issue is probably with how |
@evcohen what happens if you try |
We'll remove most uses of I suspect @freitagbr is right, the issue with |
Node version (or tell us if you're using electron or some other framework):
v6.10.2
ShellJS version (the most recent version/Github branch you see the bug on):
0.7.8
Operating system:
Mac osx
Description of the bug:
Exec function calls
JSON.stringify
on the command. This causes issues when trying to pass 64 bit unsigned long strings as arguments since JSON.stringify cannot natively handle them. They get truncated and data is lost.Is JSON.stringify necessary or can we add a flag to avoid parsing like that?
The text was updated successfully, but these errors were encountered: