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( "<div id=\"confirmationBox\" class=\"centeredContainer formBox\">" "\n" "<h2>%s</h2>", brwName); hPrintf( "<p id=\"confirmationMsg\" class=\"confirmationTxt\">A confirmation email has been sent to you. \n" "Please click the confirmation link in the email to activate your account.</p>" "<p>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.</p>" "\n" "<p><a href=\"%s\">Return</a></p>", returnURL); cartRemove(cart, "hgLogin_email"); cartRemove(cart, "hgLogin_userName"); } @@ -488,32 +535,32 @@ void sendMailOut(char *email, char *subject, char *msg) /* send username reminder email to email address */ { char *obj = cartUsualString(cart, "hgLogin_helpWith", ""); int result; result = mailViaPipeBounce(email, subject, msg, returnAddr); if (result == -1) { hPrintf( "<h2>%s</h2>", brwName); hPrintf( "<p align=\"left\">" "</p>" "<h3>Error emailing %s to: %s</h3>" - "Click <a href=%s?hgLogin.do.displayAccHelpPage=1>here</a> to return.<br>", - hgLoginUrl, obj, email ); + "Click <a href=\"%s?hgLogin.do.displayAccHelpPage=1\">here</a> to return.<br>", + 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( "<h2>%s</h2>", brwName); hPrintf( "<p align=\"left\">" "</p>" "<h3>Error emailing %s to: %s</h3>" - "Click <a href=%s?hgLogin.do.displayAccHelpPage=1>here</a> to return.<br>", - hgLoginUrl, obj, email ); + "Click <a href=\"%s?hgLogin.do.displayAccHelpPage=1\">here</a> to return.<br>", + 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("<div class=\"inputGroup\" id=\"usernameBox\" style=\"display: none;\">" "<label for=\"emailUsername\">Username</label>" "<input type=\"text\" name=\"hgLogin_userName\" value=\"%s\" size=\"30\" id=\"emailUsername\">" "</div>" "\n" "<div class=\"inputGroup\" id=\"emailAddrBox\" style=\"display: none;\">" "<label for=\"emailPassword\">Email address</label>" "<input type=\"text\" name=\"hgLogin_email\" value=\"%s\" size=\"30\" id=\"emailPassword\">" "</div>" "\n" "<div class=\"formControls\">" " <input type=\"submit\" name=\"hgLogin.do.accountHelp\" value=\"Continue\" class=\"largeButton\">" " <a href=\"%s\" class=\"cancelButton\">Cancel</a>" "</div>" "</form>" - "</div><!-- END - accountHelpBox -->", username, email, getReturnToURL()); + "</div><!-- END - accountHelpBox -->", 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 @@ "<div class=\"inputGroup\">" "<label for=\"password\">Password</label>" "<span style=\"display:inline-flex; align-items:center;\">" "<input type=password name=\"hgLogin_password\" value=\"\" size=\"30\" id=\"password\">" , hgLoginUrl, username, hgLoginUrl); printPwdEyeIcon("pwdEyeIcon", "pwdEyeSlash"); hPrintf( "</span>" "<a class=\"forgotLink\" href=\"%s?hgLogin.do.displayAccHelpPage=1&hgLogin_helpWith=password\">Forgot password</a>" "</div>" "\n" "<div class=\"formControls\">" " <input type=\"submit\" name=\"hgLogin.do.displayLogin\" value=\"Login\" class=\"largeButton\">" " <a href=\"%s\" class=\"cancelButton\">Cancel</a>" "</div>" - , hgLoginUrl, getReturnToURL()); + , hgLoginUrl, getReturnToUrlForAttr()); if (pwdEyeIconEnabled) { printPwdToggleJS(); jsOnEventById("click", "pwdEyeIcon", "togglePwdVisibility('password','pwdEyeSlash');"); } cartSaveSession(cart); hPrintf("</form>\n"); printEmailLinkButton(); printSocialButtons(TRUE, FALSE, "Sign in"); hPrintf( "</div><!-- END - loginBox -->" "\n" "\n" "</body>" "</html>"); @@ -912,31 +959,31 @@ "<span style=\"display:inline-flex; align-items:center;\">" "<input type=\"password\" name=\"hgLogin_newPassword2\" value=\"\" size=\"30\" id=\"newPw2\">"); printPwdEyeIcon("newPw2EyeIcon", "newPw2EyeSlash"); hPrintf( "</span>" "</div>" "\n" "<div class=\"formControls\">" " <input type=\"submit\" name=\"hgLogin.do.changePassword\" value=\"Change Password\" class=\"largeButton\"> " " <a href=\"%s\" class=\"cancelButton\">Cancel</a>" "\n" "</div>" "</form>" "\n" "</div><!-- END - changePwBox -->" - "\n", getReturnToURL()); + "\n", getReturnToUrlForAttr()); if (pwdEyeIconEnabled) { printPwdToggleJS(); jsOnEventById("click", "curPwEyeIcon", "togglePwdVisibility('currentPw','curPwEyeSlash');"); jsOnEventById("click", "newPw1EyeIcon", "togglePwdVisibility('newPw1','newPw1EyeSlash');"); jsOnEventById("click", "newPw2EyeIcon", "togglePwdVisibility('newPw2','newPw2EyeSlash');"); } cartSaveSession(cart); } void changePassword(struct sqlConnection *conn) /* process the change password form */ { char query[256]; char *user = cartUsualString(cart, "hgLogin_userName", ""); @@ -1130,31 +1177,31 @@ hPrintf("<div class=\"inputGroup\">" "<label for=\"curPassword\">Current password</label>" "<input type=\"password\" name=\"hgLogin_curPassword\" value=\"\" size=\"30\" id=\"curPassword\">" "</div>"); hPrintf("<div class=\"inputGroup\">" "<label for=\"newEmail1\">New email address</label>" "<input type=\"text\" name=\"hgLogin_newEmail1\" value=\"\" size=\"30\" id=\"newEmail1\">" "</div>"); hPrintf("<div class=\"inputGroup\">" "<label for=\"newEmail2\">Re-enter new email address</label>" "<input type=\"text\" name=\"hgLogin_newEmail2\" value=\"\" size=\"30\" id=\"newEmail2\">" "</div>"); hPrintf("<div class=\"formControls\">" "<input type=\"submit\" name=\"hgLogin.do.changeEmail\" value=\"Change Email\" class=\"largeButton\">" " <a href=\"%s\" class=\"cancelButton\">Cancel</a>" - "</div></form></div><!-- END - changeEmailBox -->", getReturnToURL()); + "</div></form></div><!-- END - changeEmailBox -->", getReturnToUrlForAttr()); cartSaveSession(cart); } void changeEmail(struct sqlConnection *conn) /* Process the change-email form for the currently logged-in user. */ { if (!emailLinkEnabled()) { displayLoginPage(conn); return; } char *user = wikiLinkUserName(); if (isEmpty(user)) { freez(&errMsg); @@ -1321,31 +1368,31 @@ "<span style=\"display:inline-flex; align-items:center;\">" "<input type=password name=\"hgLogin_password2\" value=\"%s\" size=\"30\" id=\"passwordCheck\">", htmlEncode(cartUsualString(cart, "hgLogin_password2", ""))); // value="" attribute; escape (XSS) printPwdEyeIcon("signupPwCheckEyeIcon", "signupPwCheckEyeSlash"); hPrintf( "</span>" "\n" "</div>" "\n" "<div class=\"formControls\">" " <input type=\"submit\" name=\"hgLogin.do.signup\" value=\"Sign Up using Email\" class=\"largeButton\"> " " <a href=\"%s\" class=\"cancelButton\">Cancel</a>" "</div>" "</form>" "</div><!-- END - signUpBox -->", - getReturnToURL()); + getReturnToUrlForAttr()); if (pwdEyeIconEnabled) { printPwdToggleJS(); jsOnEventById("click", "signupPwEyeIcon", "togglePwdVisibility('password','signupPwEyeSlash');"); jsOnEventById("click", "signupPwCheckEyeIcon", "togglePwdVisibility('passwordCheck','signupPwCheckEyeSlash');"); } cartSaveSession(cart); } void signup(struct sqlConnection *conn) /* process the signup form */ { char query[1024]; char *user = cartUsualString(cart, "hgLogin_userName", ""); char *encUserName = cgiEncodeFull(user); @@ -1951,31 +1998,31 @@ oauthProviderLabel(provider), brwName); printUsernameNote(); hPrintf("<span style='color:red;'>%s</span>", errMsg ? errMsg : ""); hPrintf("<form method=\"post\" action=\"%s\" name=\"completeAccountForm\">", hgLoginUrl); hPrintf("<div class=\"inputGroup\">" "<label for=\"userName\">Username</label>" "<input type=\"text\" name=\"hgLogin_userName\" value=\"%s\" size=\"30\" id=\"userName\">" "</div>", encSuggested); hPrintf("<div class=\"inputGroup\">" "<label for=\"emailAddr\">Email address</label>" "<input type=\"text\" name=\"hgLogin_email\" value=\"%s\" size=\"30\" id=\"emailAddr\">" "</div>", encEmail); hPrintf("<div class=\"formControls\">" "<input type=\"submit\" name=\"hgLogin.do.completeAccount\" value=\"Create Account\" class=\"largeButton\">" " <a href=\"%s\" class=\"cancelButton\">Cancel</a>" - "</div></form></div><!-- END - completeAccountBox -->", getReturnToURL()); + "</div></form></div><!-- END - completeAccountBox -->", getReturnToUrlForAttr()); cartSaveSession(cart); freeMem(encSuggested); freeMem(encEmail); } void completeAccount(struct sqlConnection *conn) /* Create the account for a first-time social-login user, link the identity, and log in. */ { char *provider = cartUsualString(cart, "oauth_pending_provider", ""); char *subject = cartUsualString(cart, "oauth_pending_subject", ""); if (isEmpty(provider) || isEmpty(subject) || !pendingIdentityValid()) { clearPendingIdentity(); freez(&errMsg); errMsg = cloneString("Your login session expired. Please sign in again."); @@ -2117,31 +2164,31 @@ boolean first = TRUE; for (m = list; m != NULL; m = m->next) { char *encUserName = htmlEncode(m->userName); hPrintf("<div class=\"acctHelpSection\">" "<input name=\"hgLogin_chosenIdx\" type=\"radio\" value=\"%u\" id=\"acct_%u\"%s>" "<label for=\"acct_%u\" class=\"radioLabel\">%s</label></div>", m->idx, m->idx, first ? " checked" : "", m->idx, encUserName); freeMem(encUserName); first = FALSE; } hPrintf("</div>"); hPrintf("<div class=\"formControls\">" "<input type=\"submit\" name=\"hgLogin.do.chooseAccount\" value=\"Sign In\" class=\"largeButton\">" " <a href=\"%s\" class=\"cancelButton\">Cancel</a>" - "</div></form></div><!-- END - chooseAccountBox -->", getReturnToURL()); + "</div></form></div><!-- END - chooseAccountBox -->", getReturnToUrlForAttr()); cartSaveSession(cart); freeMem(encEmail); gbMembersFreeList(&list); } void chooseAccount(struct sqlConnection *conn) /* Finish the "which account?" chooser: for OAuth, link the pending identity to the chosen * account; for the email link, just sign in. Either way, only accept an account that really * matches the verified email (and, for the email link, still holds the valid token), never an * arbitrary username the client might submit. */ { int chosenIdx = cartUsualInt(cart, "hgLogin_chosenIdx", 0); char *provider = cartUsualString(cart, "oauth_pending_provider", ""); char query[512]; @@ -2385,31 +2432,31 @@ "<h2>%s</h2>", brwName); hPrintf("<h3>Email me a sign-in link</h3>"); hPrintf("<p>Enter your email address and we'll send you a link that signs you in without a " "password. This is handy on a computer where you don't have your password saved.</p>"); hPrintf("<span style='color:red;'>%s</span>", errMsg ? errMsg : ""); hPrintf("<form method=\"post\" action=\"%s\" name=\"emailLinkForm\">", hgLoginUrl); char *encEmail = htmlEncode(cartUsualString(cart, "hgLogin_email", "")); hPrintf("<div class=\"inputGroup\">" "<label for=\"emailLink\">Email address</label>" "<input type=\"text\" name=\"hgLogin_email\" value=\"%s\" size=\"30\" id=\"emailLink\">" "</div>", encEmail); freeMem(encEmail); hPrintf("<div class=\"formControls\">" "<input type=\"submit\" name=\"hgLogin.do.sendEmailLink\" value=\"Send login link\" class=\"largeButton\">" " <a href=\"%s\" class=\"cancelButton\">Cancel</a>" - "</div></form></div><!-- END - emailLinkBox -->", getReturnToURL()); + "</div></form></div><!-- END - emailLinkBox -->", getReturnToUrlForAttr()); cartSaveSession(cart); } void displayLoginLinkSuccess() /* Confirmation shown after a passwordless login link is (possibly) emailed. Phrased so it * does not reveal whether an account exists for the address. */ { char *email = htmlEncode(cartUsualString(cart, "hgLogin_sendMailTo", "")); hPrintf("<div id=\"confirmationBox\" class=\"centeredContainer formBox\">" "<h2>%s</h2>", brwName); hPrintf("<p id=\"confirmationMsg\" class=\"confirmationTxt\">If an account exists for " "<B>%s</B>, a login link has been sent to that address.<BR><BR>" "Click the link in that email to sign in. No password needed. " "The link works once and expires in one hour.</p>", email); hPrintf("<p>If you don't see the email, please check your spam folder.</p>");