[PATCH v4 1/8] applets: replace exec calls with BB_EXECVP

Nadav Tasher tashernadav at gmail.com
Tue Jan 28 22:19:04 UTC 2025


Hi Ron!

Thanks for bringing this to my attention.
I will undo this change.

On Tue, Jan 28, 2025 at 10:03:38AM +0000, Ron Yorston wrote:
> Nadav Tasher <tashernadav at gmail.com> wrote:
> >--- a/networking/httpd.c
> >+++ b/networking/httpd.c
> >@@ -1706,7 +1706,7 @@ static void send_cgi_and_exit(
> > 		/* _NOT_ execvp. We do not search PATH. argv[0] is a filename
> > 		 * without any dir components and will only match a file
> > 		 * in the current directory */
> >-		execv(argv[0], argv);
> >+		BB_EXECVP(argv[0], argv);
> > 		if (verbose)
> > 			bb_perror_msg("can't execute '%s'", argv[0]);
> >  error_execing_cgi:
> 
> This change isn't appropriate:
> 
> - It doesn't work.  At this point in the code argv[0] is expected to be
>   the name of a file in the current directory, which is the cgi-bin
>   directory of the web server.  It has no directory components, so by
>   calling execvp() a path lookup is performed which (almost certainly)
>   won't find the required CGI script.
> 
> - Even if it did work it would be a very bad thing to perform a path
>   lookup up on whatever a remote user passes in a URL:
>   https://example.com/cgi-bin/whatever.  If the requested script isn't
>   in the web server's cgi-bin directory the request should fail.
> 
> I haven't done a proper review of other cases where path lookups are
> introduced by this patch set, as I'm not so familiar with the code in
> those cases.  Someone should certainly take a look at them with an eye
> towards security.
> 
> Cheers,
> 
> Ron


More information about the busybox mailing list