41e62d7a41147961651f6d317e3b90b7680d806c
braney
  Tue Aug 11 10:22:02 2026 -0700
hgMenubar: only allow incFile to name a file in the include directory, refs #38052

hgMenubar opens the file named by its incFile request parameter and prints it line
by line. The path was built as DOCUMENT_ROOT + incFile with no traversal stripping,
so a request could read any file the server can read, with no login.

Confirmed against an unpatched build. incFile=/../../../../etc/passwd returned the
password file. Pointing it at hg.conf.private returned 216 lines, 19 of which match
a password, secret or key setting, and that file is world readable so the apache
user can read it. This is credential disclosure, not just file disclosure.

incFile now has to start with /inc/ and contain no "..", otherwise it is ignored and
the normal menu bar is served. I kept the parameter rather than removing it because
printMenuBar already treats a non-default value specially, skipping printIncludes,
so an alternate include looks like a designed feature that a mirror could be using.
Nothing in our own tree passes incFile.

A rejected value is logged to stderr. Before this change a traversal attempt failed
with a 500, which was visible in the logs; without the log line the attempt would
now blend in as an ordinary page.

Falling back beats aborting here because this CGI is included into every static page
through SSI, so an abort on a bad parameter would break the page.

diff --git src/hg/hgMenubar/hgMenubar.c src/hg/hgMenubar/hgMenubar.c
index c4169b593b0..f19796e412e 100644
--- src/hg/hgMenubar/hgMenubar.c
+++ src/hg/hgMenubar/hgMenubar.c
@@ -3,30 +3,31 @@
  * On an Apache with activated SSI, a html statement like
  * <!--#include virtual="../cgi-bin/hgMenubar"-->
  * will include the menu bar into a static page.
  */
 #include "common.h"
 #include "cheapcgi.h"
 #include "dystring.h"
 #include "filePath.h"
 #include "linefile.h"
 #include "jsHelper.h"
 #include "wikiLink.h"
 #include "portable.h"
 
 #define CGI_NAME "cgi-bin/hgMenubar"
 #define NAVBAR_INC_PATH "/inc/globalNavBar.inc"
+#define NAVBAR_INC_DIR "/inc/"    /* the only directory an incFile value may name, refs #38052 */
 #define OLD_HREF "href=\"../"
 
 char* errMessage;
 
 char *loginLinkHtml()
 /* Return HTML <li> for the top-right Login menu item, or "" if no login system is configured.
  * Static-page variant: the logged-out link uses href="../cgi-bin/hgSession" so the caller's
  * OLD_HREF substitution rewrites it to a page-relative path; logged-in account-dialog URLs come
  * from wikiLink and are absolute.  topLinks.js turns the logged-in item into a dialog. */
 {
 if (!(loginSystemEnabled() || wikiLinkEnabled()))
     return cloneString("");
 struct dyString *dy = dyStringNew(512);
 char *userName = wikiLinkUserName();
 if (userName == NULL)
@@ -148,20 +149,32 @@
 
 if ( (*cgiPath == NULL) || (*docRoot == NULL) || (*pagePath == NULL) )
     {
     fprintf (stderr, "Error: bad invocation of menubar\n");
     exit (1);
     }
 }
 
 int main(int argc, char *argv[])
 /* Process command line. */
 {
 char *cgiPath, *docRoot, *pagePath;
 parseEnvOrDie(&cgiPath, &docRoot, &pagePath);
 cgiSpoof(&argc, argv);
 char *incFile = cgiUsualString("incFile", NAVBAR_INC_PATH);
+/* SECURITY (refs #38052): incFile names a server-side include that we open and print
+ * line by line, so a request must not be able to point it wherever it likes.  The path
+ * is built as docRoot + incFile with no traversal stripping, so a value like
+ * "/../../../../etc/passwd" would echo any file the server can read.  Require the known
+ * include directory and no "..".  Fall back to the normal menu bar on anything else
+ * rather than aborting, because this CGI is included into every static page and an
+ * abort would break the page instead of just ignoring a bad parameter. */
+if (!startsWith(NAVBAR_INC_DIR, incFile) || stringIn("..", incFile) != NULL)
+    {
+    fprintf(stderr, "hgMenubar: ignoring unexpected incFile value [%s]\n", incFile);
+    incFile = NAVBAR_INC_PATH;
+    }
 printMenuBar(cgiPath, docRoot, pagePath, incFile);
 if (errMessage)
     puts(errMessage);
 return 0;
 }