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

[Ron Yorston]

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