Opened 6 years ago

Closed 5 years ago

#500 closed Bug / Defect (fixed)

forced invalid path in windows

Reported by: jktseug Owned by: Gert Döring
Priority: major Milestone: release 2.3.9
Component: Generic / unclassified Version: OpenVPN 2.3.5 (Community Ed)
Severity: Not set (select this one, unless your'e a OpenVPN developer) Keywords:
Cc:

Description

In the code openvpn forces the PATH environment to the C:\ drive.
This is not correct and the OS can be installed on any drive letter.

src/openvpn/win32.c

env_block (const struct env_set *es)
{

char * force_path = "PATH=C:
Windows
System32;C:
WINDOWS;C:
WINDOWS
System32
Wbem";

Change History (10)

comment:1 Changed 6 years ago by jktseug

This is a problem at the very least in the up script, could be in the down script as well.

comment:2 Changed 6 years ago by Gert Döring

Version: 2.2.22.3.5

Patch welcome to figure out what would be "correct".

Not being a windows programmer, I did what I had to do to make the netsh.exe calls for IPv6 work (having ...\wbem in the active path) - and since I don't know anyone who installs windows anywhere but c:\windows, it never caused problems.

"Version 2.2.2" is definitely not right here, as this code didn't appear before 2.3.0

comment:3 Changed 6 years ago by jktseug

well, you could just append to the current path
PATH=%PATH%;%systemroot%\system32\wbem
should suffice to get \wbem in the path list.

at least on windows 7, wbem is in the path variable already.

You can also pull the registry
HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet?\Control\Session
Manager\Environment

this would also effect if windows is installed to something other than c:\windows
ie c:\win
c:\windowsold

This effects winpe environments where the system root is actually x:\

comment:4 Changed 5 years ago by Samuli Seppänen

Milestone: release 2.5
Owner: set to jamesyonan
Status: newassigned

Appending to the current PATH sounds reasonable. Managing the registry would probably be more hairy, as OpenVPN does not (afaik) contain any registry-managing code.

comment:5 Changed 5 years ago by Gert Döring

Milestone: release 2.5release 2.3.8
Owner: changed from jamesyonan to Gert Döring
Status: assignedaccepted

I'm not sure if it will work if I use %systemroot% inside C - that's .bat syntax to evaluate variables. Need to test.

comment:6 in reply to:  5 Changed 5 years ago by jktseug

Replying to cron2:

I'm not sure if it will work if I use %systemroot% inside C - that's .bat syntax to evaluate variables. Need to test.

It likely won't.

There are two things going on here.
adding environment variables, anytime you do that it clears out the current environment variables set up by the calling process.

After that the block needs the PATH variable to find certain commands.

If there is a reason to clear out the environmental variables (maybe some security reason, or similar) then it will have to be recreated. If not, then it should be trivial to just grab the entire environment and add the variables needed to it.

I can look a little more, if I get some time maybe I can propose a patch.

comment:7 Changed 5 years ago by jktseug

looks like std::getenv("PATH"); should work.

make sure to include the cstdlib
then

const char * force_path= std::getenv("PATH");

Version 0, edited 5 years ago by jktseug (next)

comment:8 Changed 5 years ago by Gert Döring

Milestone: release 2.3.8release 2.3.9

Uh, std::getenv() looks fairly C++'y - OpenVPN is pure C. Will that work in a C program?

comment:9 Changed 5 years ago by jktseug

http://www.tutorialspoint.com/c_standard_library/c_function_getenv.htm

I am pretty sure this is in the C standard

in fact, it should be available on every OS.

you may have to use just getenv() instead of adding the namespace to use it in C.

comment:10 Changed 5 years ago by Gert Döring

Resolution: fixed
Status: acceptedclosed

Fortunately someone came to my rescue and just sent a patch to the openvpn-devel list - thanks :-)

commit 7546cba4761b24f2195034dcd3407aecd43fd3be (master)
commit 9f87007d8997d8127b9ea5fb0cb6b6cb9c88a4f3 (release/2.3)

Author: Selva Nair
Date: Thu Nov 12 21:41:27 2015 -0500

Do not hard-code windows systemroot in env_block

merged and pushed, will show up in 2.3.9 "in the next few weeks".

Closing the ticket as I'm convinced the fix is the right thing to do (especially as inside OpenVPN, "systemroot" is actually already known - which I didn't know). If it does not work, please reopen.

Note: See TracTickets for help on using tickets.