sudo-prompt icon indicating copy to clipboard operation
sudo-prompt copied to clipboard

Issues on windows when the username contains `&`

Open zvin opened this issue 5 years ago • 13 comments

If the username contains a & (like Alice & Bob's Laptop), sudo-prompt fails on windows. Most probably because the temporary file is in the user's folder and &s are not escaped.

zvin avatar Jun 03 '19 17:06 zvin

Thanks @zvin , can you please provide any sample error messages? That will help me narrow this down.

jorangreef avatar Jun 03 '19 18:06 jorangreef

It errors here https://github.com/jorangreef/sudo-prompt/blob/master/index.js#L589 test file:

const sp = require('sudo-prompt');

const options = { name: 'test' };

sp.exec('echo ok', options, (error, stdout, stderr) => {
        console.log({ error, stdout, stderr });
});

The elevate script looks like this and fails when I run it with cmd /c ...:

@echo off
call "C:\Users\KRS&KR~1\AppData\Local\Temp\9f9c5cce3172619ac463295721953625\command.bat" > "C:\Users\KRS&KR~1\AppData\Local\Temp\9f9c5cce3172619ac463295721953625\stdout" 2> "C:\Users\KRS&KR~1\AppData\Local\Temp\9f9c5cce3172619ac463295721953625\stderr"
(echo %ERRORLEVEL%) > "C:\Users\KRS&KR~1\AppData\Local\Temp\9f9c5cce3172619ac463295721953625\status"

zvin avatar Jun 04 '19 00:06 zvin

Thanks @zvin, I could not reproduce any error.

To make sure it's not a 8.3 short name issue, I created a directory called k&ritchie, and referenced it using its 8.3 short name of K&RITC~1.

I then created elevate.bat as follows:

@echo off
call "C:\Users\Joran\sudo-prompt\K&RITC~1\command.bat" > "C:\Users\Joran\sudo-prompt\K&RITC~1\stdout" 2> "C:\Users\Joran\sudo-prompt\K&RITC~1\stderr"
(echo %ERRORLEVEL%) > "C:\Users\Joran\sudo-prompt\K&RITC~1\status"

This should be exactly the same as your example, except with a different 8.3 short name.

command.bat:

echo ok

I then ran this with:

cmd /c elevate.bat

Everything ran smoothly, the status code returned was 0, and the stdout file contained ok.

Also, the example you posted looks like it is properly escaped.

Are you sure this is not an issue with your command itself?

jorangreef avatar Jun 04 '19 13:06 jorangreef

I can reproduce the issue reliably. I must have been wrong in my diagnosis above. It is throwing an error here https://github.com/jorangreef/sudo-prompt/blob/master/index.js#L589 because the stdout, stderr and status files do not exist (I disabled the removal of temp files and checked in the folder). The powershell command looks like

powershell.exe Start-Process -FilePath "'C:\Users\KRS&KR~1\AppData\Local\Temp\81a1d4081334e8e3ac6906e9bd180f37\execute.bat'" -WindowStyle hidden -Verb runAs

Launching it by hand does not create the stdout, stderr and status files.

zvin avatar Jun 04 '19 14:06 zvin

The command itself is echo ok, my whole script is in my comment above.

zvin avatar Jun 04 '19 14:06 zvin

I can reproduce it now.

Ignoring anything specific to sudo-prompt and PowerShell, here's a simple VBS script which will run a file in the same directory called elevate.bat with administrator permissions.

Set objShell = CreateObject("Shell.Application")
objShell.ShellExecute "elevate.bat", "", "", "runas", 0

Just save this as elevate.vbs and put it in the same directory as elevate.bat.

The interesting thing here is we are relying on the current working directory to pass the full path to the elevate.bat script. This leaves all escaping to Windows.

If the full path to these files has no ampersand, everything will work.

But, if the full path includes an ampersand, Windows itself won't escape the ampersand when ShellExecute expands the path to elevate.bat.

jorangreef avatar Jun 04 '19 18:06 jorangreef

Whether we elevate using VBScript or PowerShell, the issue is actually in runAs with both of them, not with sudo-prompt.

We are passing the path correctly here: https://github.com/jorangreef/sudo-prompt/blob/master/index.js#L523

But PowerShell is not escaping the path when it proceeds to call cmd.exe /C itself later (you can see what PowerShell does by clicking "Show more details" in the permissions prompt UI dialog, you can actually see the full command and path constructed by PowerShell itself there).

I don't know how we can do the escaping for PowerShell. If we try, PowerShell doesn't seem to find the file we are trying to run (I think it stats it first before calling cmd.exe).

The only way seems to be to put the elevate.bat in a path somewhere on the system without ampersands. Any ideas? These will probably all be restricted paths.

Maybe you can give the escaping a shot to see if you come up with another solution?

Alternatively, in your case, you might try not using the 8.3 short name, i.e. use the long path, since that will probably not have any ampersands in it. We could also try something where if we see the path is an 8.3 short name, then we find a way to expand it (any ideas?), to get rid of the ampersand.

jorangreef avatar Jun 04 '19 18:06 jorangreef

The ampersand is in the user name so it is also in the name of his home folder, short names only add the ~.

zvin avatar Jun 05 '19 11:06 zvin

@bitcrazed could we get some advice from your team on this? The issue is that an ampersand in the username is breaking the script for both powershell and vbscript, see https://github.com/jorangreef/sudo-prompt/issues/97#issuecomment-498784335

Tyriar avatar Jun 05 '19 13:06 Tyriar

@bitcrazed, the issue seems to be something internal to powershell and vbscript, since we're letting them do the escaping of the ampersand in https://github.com/jorangreef/sudo-prompt/issues/97#issuecomment-498784335, by referencing a file in the same cwd. It's only the full path to the cwd that contains the ampersand. The idea behind this was to rule out any escaping errors on our side.

jorangreef avatar Jun 19 '19 11:06 jorangreef

Ping @bitcrazed, I'm pretty sure this is a bug in Windows' shell escaping...

If you can put anyone else at Microsoft onto this it would be much appreciated.

jorangreef avatar Aug 12 '19 06:08 jorangreef

Hey @jorangreef - could I ask you to file an issue in https://github.com/microsoft/terminal/ referencing this issue? We'll triage and comment there.

bitcrazed avatar Aug 12 '19 20:08 bitcrazed

Thanks @bitcrazed, done: https://github.com/microsoft/terminal/issues/2419

jorangreef avatar Aug 13 '19 12:08 jorangreef