On 9/24/24 8:26 AM, i...@apache.org wrote: > Author: ivan > Date: Tue Sep 24 06:26:21 2024 > New Revision: 1920871 > > URL: http://svn.apache.org/viewvc?rev=1920871&view=rev > Log: > apr_proc_create(): Check that progname argument is quoted correctly if > it's quoted on Windows. > > Modified: > apr/apr/trunk/CHANGES > apr/apr/trunk/test/testproc.c > apr/apr/trunk/threadproc/win32/proc.c > > Modified: apr/apr/trunk/CHANGES > URL: > http://svn.apache.org/viewvc/apr/apr/trunk/CHANGES?rev=1920871&r1=1920870&r2=1920871&view=diff > ============================================================================== > --- apr/apr/trunk/CHANGES [utf-8] (original) > +++ apr/apr/trunk/CHANGES [utf-8] Tue Sep 24 06:26:21 2024 > @@ -297,6 +297,9 @@ Changes for APR 2.0.0 > *) apr_proc_create(): Fix potential handle leak when apr_proc_create() is > used > from multiple threads on Windows [Ivan Zhakov] > > + *) apr_proc_create(): Check that progname argument is quoted correctly if > + it's quoted on Windows. [Ivan Zhakov] > + > Changes for APR and APR-util 1.7.x and later: > > *) http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/CHANGES?view=markup > > Modified: apr/apr/trunk/test/testproc.c > URL: > http://svn.apache.org/viewvc/apr/apr/trunk/test/testproc.c?rev=1920871&r1=1920870&r2=1920871&view=diff > ============================================================================== > --- apr/apr/trunk/test/testproc.c (original) > +++ apr/apr/trunk/test/testproc.c Tue Sep 24 06:26:21 2024 > @@ -300,6 +300,34 @@ static void test_proc_args_winbatch(abts > ABTS_STR_EQUAL(tc, expected, actual); > } > > +#ifdef WIN32 > +static void test_proc_unclosed_quote1(abts_case *tc, void *data) > +{ > + apr_procattr_t *attr; > + apr_status_t rv; > + const char *args[] = { NULL }; > + > + rv = apr_procattr_create(&attr, p); > + ABTS_INT_EQUAL(tc, APR_SUCCESS, rv); > + > + rv = apr_proc_create(&newproc, "\"", args, NULL, attr, p); > + ABTS_INT_EQUAL(tc, APR_EINVAL, rv); > +} > + > +static void test_proc_unclosed_quote2(abts_case *tc, void *data) > +{ > + apr_procattr_t *attr; > + apr_status_t rv; > + const char *args[] = { NULL }; > + > + rv = apr_procattr_create(&attr, p); > + ABTS_INT_EQUAL(tc, APR_SUCCESS, rv); > + > + rv = apr_proc_create(&newproc, "\"abc", args, NULL, attr, p); > + ABTS_INT_EQUAL(tc, APR_EINVAL, rv); > +} > +#endif > + > abts_suite *testproc(abts_suite *suite) > { > suite = ADD_SUITE(suite) > @@ -311,6 +339,8 @@ abts_suite *testproc(abts_suite *suite) > abts_run_test(suite, test_proc_args, NULL); > #ifdef WIN32 > abts_run_test(suite, test_proc_args_winbatch, NULL); > + abts_run_test(suite, test_proc_unclosed_quote1, NULL); > + abts_run_test(suite, test_proc_unclosed_quote2, NULL); > #endif > > return suite; > > Modified: apr/apr/trunk/threadproc/win32/proc.c > URL: > http://svn.apache.org/viewvc/apr/apr/trunk/threadproc/win32/proc.c?rev=1920871&r1=1920870&r2=1920871&view=diff > ============================================================================== > --- apr/apr/trunk/threadproc/win32/proc.c (original) > +++ apr/apr/trunk/threadproc/win32/proc.c Tue Sep 24 06:26:21 2024 > @@ -509,7 +509,16 @@ APR_DECLARE(apr_status_t) apr_proc_creat > * XXX progname must be NULL if this is a 16 bit app running in WOW > */ > if (progname[0] == '\"') { > - progname = apr_pstrmemdup(pool, progname + 1, strlen(progname) - 2); > + size_t progname_len = strlen(progname); > + if (progname_len < 2) { > + return APR_EINVAL; > + } > + > + if (progname[progname_len - 1] != '\"') { > + return APR_EINVAL; > + } Shouldn't we ensure that there is no '\"' in the middle of the string, e.g if (strchr(progname + 1, '\"') != progname + progname_len - 1) { return APR_EINVAL; } ? Regards Rüdiger
