Re: cygrunsrv CWE-428

Hi James,

thanks for the patch.  One point, though:

On Apr 21 08:57, James Warnock via Cygwin wrote:
> diff --git a/cygrunsrv.cc b/cygrunsrv.cc
> index dab8790..c3b04ee 100644
> --- a/cygrunsrv.cc
> +++ b/cygrunsrv.cc
> @@ -810,6 +810,7 @@ install_service (const char *name, const char *crspath, const char *disp,
>  		 int interactive)
>  {
>    char mypath[MAX_PATH];
> +  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,28 +825,31 @@ 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-2);
                                                                  ^^^^^^^^^^

>        if (strcasecmp (mypath + strlen (mypath) - 4, ".exe") != 0)
>          strcat (mypath, ".exe");
>      }
>    else if (san.server ()) /* Figure out cygrunsrv path on remote server. */
>      {
> -      DWORD ret, type, size = MAX_PATH - 20;
> +      DWORD ret, type, size = MAX_PATH - 22;
                                 ^^^^^^^^^^^^^

Rather than further restricting the maximum path length, wouldn't it
make sense to raise the size of mypath to MAX_PATH + 2?


Thanks,
Corinna

-- 
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