67f89eb6f4f5e8ca51c551cdf0918a2cc33ad864
braney
Fri Aug 21 09:01:22 2026 -0700
hgLogin: tighten validation and encoding of the return URL
Validate the return URL in one place, in getReturnToURL: accept only http, https
or a relative URL, and refuse characters that a properly encoded URL never
contains. The login.approvedReturn host check is unchanged and stays optional, so
a mirror that never set it behaves as before.
Encode the value where it is written out: html encoding in the href attributes,
javaScriptLiteralEncode in the location assignment.
Also fix the argument order in the two mail-failure messages, which was rotated
by one, so the values land where the format string means them to, and encode the
user name put into the mail-success redirect.
refs #38011
diff --git src/hg/hgLogin/hgLogin.c src/hg/hgLogin/hgLogin.c
index 7fe4b2d549b..54a2715a332 100644
--- src/hg/hgLogin/hgLogin.c
+++ src/hg/hgLogin/hgLogin.c
@@ -342,76 +342,123 @@
{
struct slName *approvedHosts = slNameListFromComma(cfgOptionDefault(CFG_APPROVED_HOSTS, NULL));
slAddHead(&approvedHosts, slNameNew(hLoginHostCgiBinUrl()));
if (approvedHosts)
{
struct slName *approvedStart;
for (approvedStart = approvedHosts; approvedStart != NULL; approvedStart = approvedStart->next)
{
if (startsWith(approvedStart->name, returnUrl))
return TRUE;
}
}
return FALSE;
}
+static boolean returnUrlSchemeIsSafe(char *returnUrl)
+/* Return TRUE unless returnUrl carries a scheme other than http or https. The scheme is the
+ * text before the first colon, and only when that colon comes before any slash, question mark
+ * or hash; a colon after one of those belongs to the path or the query, so the URL is relative.
+ * This is what keeps a javascript: or data: URL out of the href we write. */
+{
+char *colon = strchr(returnUrl, ':');
+if (colon == NULL)
+ return TRUE;
+char *pathStart = strpbrk(returnUrl, "/?#");
+if (pathStart != NULL && pathStart < colon)
+ return TRUE;
+int schemeLen = colon - returnUrl;
+return (schemeLen == 4 && startsWithNoCase("http", returnUrl))
+ || (schemeLen == 5 && startsWithNoCase("https", returnUrl));
+}
+
+static boolean returnUrlIsWellFormed(char *returnUrl)
+/* Return TRUE if returnUrl looks like a URL we can write into the page. Every CGI parameter
+ * becomes a cart variable, so returnto holds whatever the visitor's URL said, and it is printed
+ * into an href attribute and into a javascript location assignment. A quote, an angle bracket,
+ * a backslash or a control character would end the attribute or the string literal and reflect
+ * script onto the page. A real URL percent-encodes all of those, so refusing them turns away
+ * nothing legitimate. */
+{
+char *c;
+for (c = returnUrl; *c != 0; c++)
+ {
+ unsigned char uc = (unsigned char)*c;
+ if (uc < ' ' || uc == 127 || strchr("\"'<>\\`", *c) != NULL)
+ return FALSE;
+ }
+return returnUrlSchemeIsSafe(returnUrl);
+}
+
char *getReturnToURL()
/* get URL from cart var returnto; if empty, make URL to hgSession on login host. */
{
char *returnURL = cartUsualString(cart, "returnto", "");
char returnTo[2048];
-if (!returnURL || sameString(returnURL,""))
+if (isEmpty(returnURL))
safef(returnTo, sizeof(returnTo), "%shgSession?hgS_doMainPage=1", hLoginHostCgiBinUrl());
-else if (cfgOptionDefault(CFG_APPROVED_HOSTS, NULL))
- {
- if (isValidReturnUrl(returnURL))
- safecpy(returnTo, sizeof(returnTo), returnURL);
else
+ {
+ /* Check the shape of the URL on every install. login.approvedReturn is optional, and
+ * where it is set it only matches the front of the URL, so the rest of the URL is
+ * unchecked either way. */
+ boolean ok = returnUrlIsWellFormed(returnURL);
+ if (ok && cfgOptionDefault(CFG_APPROVED_HOSTS, NULL))
+ ok = isValidReturnUrl(returnURL);
+ if (!ok)
{
hDumpStackDisallow();
errAbort("Error: Invalid returnto URL. Please send email to genome-www@soe.ucsc.edu "
"with the returnto argument from the URL (or just the full URL) so we can "
"fix this.");
}
- }
-else
safecpy(returnTo, sizeof(returnTo), returnURL);
+ }
return cloneString(returnTo);
}
+static char *getReturnToUrlForAttr()
+/* getReturnToURL() escaped for printing inside an href="" attribute. Escaping the ampersand
+ * is the part that matters here: the browser expands an entity in an attribute value, so
+ * javascript:alert(1) would otherwise become a javascript: URL after the checks above
+ * have passed it. */
+{
+return htmlEncode(getReturnToURL());
+}
+
void returnToURL(int delay)
/* delay for delay mill-seconds then return to the "returnto" URL */
{
-char *returnURL = getReturnToURL();
+char *returnURL = javaScriptLiteralEncode(getReturnToURL());
jsInlineF(
"setTimeout(function(){location='%s';}, %d);\n"
, returnURL, delay);
}
static void redirectToLoginPage(char *paramStr)
/* redirect to hgLogin page with given parameter string */
{
jsInlineF(
"window.location ='%s?%s';\n"
, hgLoginUrl, paramStr);
}
void displayActMailSuccess()
/* display Activate mail success box */
{
-char *returnURL = getReturnToURL();
+char *returnURL = getReturnToUrlForAttr();
hPrintf(
"
"
"\n"
"
%s
", brwName);
hPrintf(
"
A confirmation email has been sent to you. \n"
"Please click the confirmation link in the email to activate your account.
"
"
You may have to look in your spam folder for an email from genome-www@soe.ucsc.edu, "
"especially if you use Microsoft Outlook or Hotmail.
"
- "Click here to return. ",
- hgLoginUrl, obj, email );
+ "Click here to return. ",
+ htmlEncode(obj), htmlEncode(email), hgLoginUrl );
}
else
{
jsInlineF(
"window.location = '%s?hgLogin.do.displayMailSuccess=1';\n"
, hgLoginUrl);
}
}
void mailUsername(char *email, char *users)
/* send user name list to the email address */
{
char subject[256];
char msg[4096];
char *remoteAddr=getenv("REMOTE_ADDR");
@@ -555,38 +602,38 @@
char *obj = cartUsualString(cart, "hgLogin_helpWith", "");
int result;
result = mailViaPipeBounce(email, subject, msg, returnAddr);
if ((result != -1) && !isEmpty(recovEmail))
result = mailViaPipeBounce(recovEmail, subject, msg, returnAddr);
if (result == -1)
{
hPrintf(
"
%s
", brwName);
hPrintf(
"
"
"
"
"
Error emailing %s to: %s
"
- "Click here to return. ",
- hgLoginUrl, obj, email );
+ "Click here to return. ",
+ htmlEncode(obj), htmlEncode(email), hgLoginUrl );
}
else
{
jsInlineF(
"window.location = '%s?hgLogin.do.displayMailSuccessPwd=1&user=%s';\n"
- , hgLoginUrl, username);
+ , hgLoginUrl, cgiEncodeFull(username));
}
}
void sendNewPwdMail(char *username, char *email, char *recovEmail, char *password)
/* send user new password */
{
char subject[256];
char msg[4096];
char *remoteAddr=getenv("REMOTE_ADDR");
safef(subject, sizeof(subject),"New temporary password for your account at the %s", brwName);
safef(msg, sizeof(msg),
" Someone (probably you, from IP address %s) requested a new password for the %s (%s). A temporary password for user \"%s\" has been created and was set to \"%s\". If this was your intent, you will need to log in and choose a new password now. Your temporary password will expire in 7 days.\n\n If someone else made this request, or if you have remembered your password, and you no longer wish to change it, you may ignore this message and continue using your old password.\n\n%s\n%s",
remoteAddr, brwName, brwAddr, username, password, signature, returnAddr);
sendPwdMailOut(email, recovEmail, subject, msg, username);
@@ -636,31 +683,31 @@
hPrintf("
", username, email, getReturnToURL());
+ "", username, email, getReturnToUrlForAttr());
jsOnEventById("click", "password", "toggle('showU');");
jsOnEventById("click", "username", "toggle('showE');");
if (emailLinkEnabled())
jsOnEventById("click", "loginLink", "toggle('showE');");
// If a radio was preselected via the URL, open its matching input box on load.
if (sameString(pre, "password"))
jsInline("toggle('showU');");
else if (sameString(pre, "username") || sameString(pre, "loginLink"))
jsInline("toggle('showE');");
cartSaveSession(cart);
}
void sendNewPassword(struct sqlConnection *conn, char *username, char *password)
/* email user new password */
{
@@ -809,31 +856,31 @@
"