[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