Re: cygrunsrv CWE-428

James Warnock via Cygwin <[email protected]> Thu, 23 Apr 2026 10:04:40 -0600
Newsgroups gmane.os.cygwin
Message-ID <[email protected]> 
On 4/22/26 02:29, Corinna Vinschen via Cygwin wrote:

> Most of the time it's just a permission issue.  You have to have
> (domain) admin perms on the remote machine and you have to run in an
> elevated shell.

I didn't have a domain set up so I had to do that. Once I got it all
up and running, The remote service install progressed further but
failed to open the key. When I checked what key it was opening it was:

"SOFTWARE\\Cygnus Solutions\\Cygwin\\mounts v2\\/"

After some research, it appears that might be an old key. I changed it
to:

"SOFTWARE\\Cygwin\\setup"

And changed the value name from "native" to "rootdir". With those
changes, the remote install worked. I don't know if I did something
wrong and that necessitated those extra changes, but I think my test
still counts.

My changes are based on master with the commit hash
bfc6c36a052914cd4aa93d883e73a2964969e4e5. Maybe I'm on the wrong
repository or commit and that is why I am seeing the old key.
Hopefully I'm not working from the wrong place and just wasting your
time.

Anyway, the new version of the patch is attached (hopefully correctly).
It increases the buffer size to include the quotes rather than limiting
the path size to accommodate the quotes. I thought about trying to
handle buffer overflows, but since none of the other strcat calls did
that, I decided to hold off.

Thanks,
James


-- 
Problem reports:      https://cygwin.com/problems.html
FAQ:                  https://cygwin.com/faq/
Documentation:        https://cygwin.com/docs.html
Unsubscribe info:     https://cygwin.com/ml/#unsubscribe-simple
cwe-428.patch (text/x-patch, 1.8 KB) 
diff --git a/cygrunsrv.cc b/cygrunsrv.cc
index dab8790..6951244 100644
--- a/cygrunsrv.cc
+++ b/cygrunsrv.cc
@@ -809,7 +809,8 @@ install_service (const char *name, const char *crspath, const char *disp,
 		 type_t type, char *user, char *pass, char **deps,
 		 int interactive)
 {
-  char mypath[MAX_PATH];
+  char mypath[MAX_PATH+2];
+  char* mypath_p = mypath;
   SC_HANDLE sm = (SC_HANDLE) 0;
   SC_HANDLE sh = (SC_HANDLE) 0;
   char userbuf[INTERNET_MAX_HOST_NAME_LENGTH + UNLEN + 2];
@@ -824,9 +825,11 @@ install_service (const char *name, const char *crspath, const char *disp,
   if (!san.server ())
     check_system_mounts ();
 
+  mypath[0] = '"';
+  mypath_p++;
   if (crspath)		  /* Got path, nothing to do. */
     {
-      cygwin_conv_path (CCP_POSIX_TO_WIN_A, crspath, mypath, MAX_PATH);
+      cygwin_conv_path (CCP_POSIX_TO_WIN_A, crspath, mypath_p, MAX_PATH);
       if (strcasecmp (mypath + strlen (mypath) - 4, ".exe") != 0)
         strcat (mypath, ".exe");
     }
@@ -837,15 +840,16 @@ install_service (const char *name, const char *crspath, const char *disp,
 			       KEY_READ, &cyg_root_key)) != ERROR_SUCCESS)
 	err_out_set_error (RegOpenKeyEx, ret);
       if ((ret = RegQueryValueEx (cyg_root_key, (PCHAR) CYG_ROOT_VAL, 0, &type,
-				  (BYTE *) mypath, &size)) != ERROR_SUCCESS)
+				  (BYTE *) mypath_p, &size)) != ERROR_SUCCESS)
 	err_out_set_error (RegQueryValueEx, ret);
-      strcat (mypath, "\\bin\\cygrunsrv.exe");
+      strcat (mypath_p, "\\bin\\cygrunsrv.exe");
     }
   else			  /* Get own full path. */
     {
-      if (!GetModuleFileName (NULL, mypath, MAX_PATH))
+      if (!GetModuleFileName (NULL, mypath_p, MAX_PATH))
 	err_out (GetModuleFileName);
     }
+  strcat (mypath_p, "\"");
   /* Open service manager database. */
   if (!(sm = OpenSCManager (san.server (), NULL,
 			    SC_MANAGER_CONNECT | SC_MANAGER_CREATE_SERVICE)))