Skip to content
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

child_process execFile / spawn throw non-descript exception on windows if exe requires elevation #9464

Closed
TanninOne opened this issue Nov 4, 2016 · 7 comments
Labels
child_process Issues and PRs related to the child_process subsystem. libuv Issues and PRs related to the libuv dependency or the uv binding. windows Issues and PRs related to the Windows platform.

Comments

@TanninOne
Copy link

TanninOne commented Nov 4, 2016

  • Version: v6.5.0
  • Platform: Windows 10 (x64)
  • Subsystem: child_process
let cp = require('child_process');
let util = require('util');
try {
  cp.execFileSync('SomeTool.exe')
} catch (err) {
  console.log('err', util.inspect(err));
}

outputs

err { Error: spawnSync SomeTool.exe UNKNOWN
    at exports._errnoException (util.js:1026:11)
    at spawnSync (child_process.js:461:20)
    at Object.execFileSync (child_process.js:498:13)
    at repl:2:4
    at sigintHandlersWrap (vm.js:22:35)
    at sigintHandlersWrap (vm.js:96:12)
    at ContextifyScript.Script.runInThisContext (vm.js:21:12)
    at REPLServer.defaultEval (repl.js:313:29)
    at bound (domain.js:280:14)
    at REPLServer.runBound [as eval] (domain.js:293:12)
  code: 'UNKNOWN',
  errno: 'UNKNOWN',
  ...
}

spawn generates the same error.

Obviously it would be better if this error was returned through the callback but I would also love to have a proper error code to react to the situation.
Not sure how execFile and spawn are implemented but it should be possible to report a proper errorcode as CreateProcess generates errorcode 740 if elevation is required.

@mscdex mscdex added child_process Issues and PRs related to the child_process subsystem. libuv Issues and PRs related to the libuv dependency or the uv binding. windows Issues and PRs related to the Windows platform. labels Nov 4, 2016
@silverwind
Copy link
Contributor

Does SomeTool.exe actually exit with this error level? Try in PowerShell:

SomeTool.exe
echo %ERRORLEVEL%

@TanninOne
Copy link
Author

It doesn't work like this. As I said: SomeTool.exe requires elevation, which means if you start it in cmd.exe or powershell it displays a UAC dialog and if the user accepts it the application is run in a separate administrator shell. The orignal shell doesn't receive an exit code.

However, running execFile/spawn from node does not display an UAC dialog to begin with so SomeTool.exe isn't started at all. It can't be - unless node exploits a security bug in Windows to circumvent security measures. Whatever api node/libuv is using to spawn a process (I assume CreateProcess because ShellExecute would work and display a UAC dialog) fails immediately with an error code that says the user doesn't have the permission to run that exe and node doesn't report that error code back to the application.

I just updated to node 6.9.1 to repeat the test. I also realized that using execFileSync might hide an aspect of the problem:

let cp = require('child_process');
let util = require('util');
try {
  cp.execFile('SomeTool.exe', (err, out) => {
    if (err) {
      console.log('err', err);
    }
  });
} catch (ex) {
  console.log('exception', ex);
}

reports:

exception { Error: spawn UNKNOWN
    at exports._errnoException (util.js:1026:11)
    at ChildProcess.spawn (internal/child_process.js:313:11)
    at exports.spawn (child_process.js:380:9)
    ...

As you see, the error is reported synchronously as an exception, not as an error through the callback.

@silverwind
Copy link
Contributor

FYI, It's using CreateProcess.

@richardlau
Copy link
Member

UNKNOWN is because ERROR_ELEVATION_REQUIRED (740) is not translated in uv_translate_sys_error(). I'll submit a fix upstream.

richardlau added a commit to richardlau/libuv that referenced this issue Nov 30, 2016
uv_spawn() on Windows will fail with ERROR_ELEVATION_REQUIRED (740)
if attempting to run an application that requires elevation.

Add a Windows test that calls uv_spawn() with a helper .exe requires
elevated permissions. Test should either run successfully if run with
Administrator privileges or fail with the mapped error.

Refs: nodejs/node#9464
richardlau added a commit to richardlau/libuv that referenced this issue Nov 30, 2016
uv_spawn() on Windows will fail with ERROR_ELEVATION_REQUIRED (740) if
attempting to run an application that requires elevation.

Add a Windows test that calls uv_spawn() with a helper .exe that requires
elevated permissions. Test should either run successfully if run with
Administrator privileges or fail with the mapped error.

Refs: nodejs/node#9464
richardlau added a commit to richardlau/libuv that referenced this issue Nov 30, 2016
uv_spawn() on Windows will fail with ERROR_ELEVATION_REQUIRED (740) if
attempting to run an application that requires elevation.

Add a Windows test that calls uv_spawn() with a helper .exe that
requires elevated permissions. Test should either run successfully if
run with Administrator privileges or fail with the mapped error.

Refs: nodejs/node#9464
richardlau added a commit to richardlau/libuv that referenced this issue Nov 30, 2016
uv_spawn() on Windows will fail with ERROR_ELEVATION_REQUIRED (740) if
attempting to run an application that requires elevation.

Add a Windows test that calls uv_spawn() with a helper .exe that
requires elevated permissions. Test should either run successfully if
run with Administrator privileges or fail with the mapped error.

Refs: nodejs/node#9464
@richardlau
Copy link
Member

libuv/libuv#1154

richardlau added a commit to richardlau/libuv that referenced this issue Dec 1, 2016
uv_spawn() on Windows will fail with ERROR_ELEVATION_REQUIRED (740) if
attempting to run an application that requires elevation.

Add a Windows test that calls uv_spawn() with a helper .exe that
requires elevated permissions. Test should either run successfully if
run with Administrator privileges or fail with the mapped error.

Refs: nodejs/node#9464
richardlau added a commit to richardlau/libuv that referenced this issue Dec 1, 2016
uv_spawn() on Windows will fail with ERROR_ELEVATION_REQUIRED (740) if
attempting to run an application that requires elevation.

Add a Windows test that calls uv_spawn() with a helper .exe that
requires elevated permissions. Test should either run successfully if
run with Administrator privileges or fail with the mapped error.

Refs: nodejs/node#9464
richardlau added a commit to richardlau/libuv that referenced this issue Dec 1, 2016
uv_spawn() on Windows will fail with ERROR_ELEVATION_REQUIRED (740) if
attempting to run an application that requires elevation.

Add a Windows test that calls uv_spawn() with a helper .exe that
requires elevated permissions. Test should either run successfully if
run with Administrator privileges or fail with the mapped error.

Refs: nodejs/node#9464
richardlau added a commit to richardlau/libuv that referenced this issue Dec 1, 2016
uv_spawn() on Windows will fail with ERROR_ELEVATION_REQUIRED (740) if
attempting to run an application that requires elevation.

Add a Windows test that calls uv_spawn() with a helper .exe that
requires elevated permissions. Test should either run successfully if
run with Administrator privileges or fail with the mapped error.

Refs: nodejs/node#9464
richardlau added a commit to richardlau/libuv that referenced this issue Dec 5, 2016
uv_spawn() on Windows will fail with ERROR_ELEVATION_REQUIRED (740) if
attempting to run an application that requires elevation.

Refs: nodejs/node#9464
richardlau added a commit to richardlau/libuv that referenced this issue Dec 5, 2016
uv_spawn() on Windows will fail with ERROR_ELEVATION_REQUIRED (740) if
attempting to run an application that requires elevation.

Add a Windows test that calls uv_spawn() with a helper .exe that
requires elevated permissions. Test should either run successfully if
run with Administrator privileges or fail with the mapped error.

Refs: nodejs/node#9464
richardlau added a commit to richardlau/libuv that referenced this issue Dec 14, 2016
uv_spawn() on Windows will fail with ERROR_ELEVATION_REQUIRED (740) if
attempting to run an application that requires elevation.

Refs: nodejs/node#9464
@richardlau
Copy link
Member

Should this remain open until Node.js is updated to a version of libuv containing the fix?

@silverwind silverwind reopened this Dec 16, 2016
@silverwind
Copy link
Contributor

Yes, I'd say so.

cjihrig added a commit to cjihrig/node that referenced this issue Jan 12, 2017
Refs: nodejs#9439
Fixes: nodejs#9464
Fixes: nodejs#9690
PR-URL: nodejs#10717
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
jBarz pushed a commit to ibmruntimes/libuv-zos that referenced this issue Jan 17, 2017
uv_spawn() on Windows will fail with ERROR_ELEVATION_REQUIRED
if attempting to run an application that requires elevation.

Fixes: nodejs/node#9464
PR-URL: libuv#1154
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 18, 2017
Refs: nodejs#9439
Fixes: nodejs#9464
Fixes: nodejs#9690
PR-URL: nodejs#10717
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 23, 2017
Refs: nodejs#9439
Fixes: nodejs#9464
Fixes: nodejs#9690
PR-URL: nodejs#10717
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 25, 2017
Refs: nodejs#9439
Fixes: nodejs#9464
Fixes: nodejs#9690
PR-URL: nodejs#10717
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 27, 2017
Refs: nodejs#9439
Fixes: nodejs#9464
Fixes: nodejs#9690
PR-URL: nodejs#10717
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
MylesBorins pushed a commit that referenced this issue May 16, 2017
Refs: #9439
Fixes: #9464
Fixes: #9690
PR-URL: #10717
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
MylesBorins pushed a commit that referenced this issue May 18, 2017
Refs: #9439
Fixes: #9464
Fixes: #9690
PR-URL: #10717
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
andrew749 pushed a commit to michielbaird/node that referenced this issue Jul 19, 2017
Refs: nodejs/node#9439
Fixes: nodejs/node#9464
Fixes: nodejs/node#9690
PR-URL: nodejs/node#10717
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
child_process Issues and PRs related to the child_process subsystem. libuv Issues and PRs related to the libuv dependency or the uv binding. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

No branches or pull requests

4 participants