516b59c2d55af8ec1d71f43c06e545f5633b4cd3
max
  Tue Aug 11 06:15:09 2026 -0700
hgLogin: fix two social-login gaps found in v502 code review, refs #38037

#1 The OAuth account-chooser query listed accounts the action would then
refuse: it lacked the accountActivated='Y' filter that chooseAccount() and
resolveIdentity() both apply, so the page could offer an unactivated row (and
expose the username anyone could have created with the victim's address). Add
the filter so the page and the action agree.

#2 oauthReturn() removed oauth_state but never oauth_provider, and it acted on
the error parameter before checking state. So after one social login a later
hgLogin?error=<text> re-entered the flow and printed the (escaped) provider
text on the login page, and a crafted error link could consume the state nonce
of a login in flight. Validate state first, then clear both oauth_state and
oauth_provider, so the flow ends cleanly and a stray code/error link is ignored.

Found in the v502 final-build code review, refs #38069.

diff --git src/hg/hgLogin/hgLogin.c src/hg/hgLogin/hgLogin.c
index 216aeb150d3..c16fd4b7a7f 100644
--- src/hg/hgLogin/hgLogin.c
+++ src/hg/hgLogin/hgLogin.c
@@ -2073,32 +2073,35 @@
     {
     if (!emailMode)
         clearPendingIdentity();
     displayLoginPage(conn);
     return;
     }
 char *encEmail = htmlEncode(email);   // the address is displayed; never trust it raw (XSS)
 char query[512];
 if (emailMode)
     // Only the accounts that hold the just-validated login token, matching what emailLogin saw.
     sqlSafef(query, sizeof(query),
         "SELECT * FROM gbMembers WHERE (email='%s' OR recovEmail='%s') AND loginToken='%s' "
         "AND loginToken<>'' AND loginTokenExpires > NOW() AND accountActivated='Y' ORDER BY idx",
         email, email, cartUsualString(cart, "emailLogin_tokenMd5", ""));
 else
+    // Only activated accounts, matching what chooseAccount() and resolveIdentity() accept;
+    // otherwise the page offers a row the action refuses, and shows the username of an
+    // unactivated row anyone could have created with this address.
     sqlSafef(query, sizeof(query),
-        "SELECT * FROM gbMembers WHERE email='%s' ORDER BY idx", email);
+        "SELECT * FROM gbMembers WHERE email='%s' AND accountActivated='Y' ORDER BY idx", email);
 struct gbMembers *list = gbMembersLoadByQuery(conn, query), *m;
 
 hPrintf("<div id=\"chooseAccountBox\" class=\"centeredContainer formBox\">"
     "<h2>%s</h2>", brwName);
 hPrintf("<h3>Choose an account</h3>");
 if (emailMode)
     hPrintf("<p>The email address <b>%s</b> is associated with more than one %s account. "
         "Select the account you would like to sign in to.</p>", encEmail, brwName);
 else
     hPrintf("<p>The email address <b>%s</b> is associated with more than one %s account. "
         "Select the account you would like to sign in to; your %s login will be linked to it.</p>",
         encEmail, brwName, oauthProviderLabel(provider));
 hPrintf("<span style='color:red;'>%s</span>", errMsg ? errMsg : "");
 hPrintf("<form method=\"post\" action=\"%s\" name=\"chooseAccountForm\">", hgLoginUrl);
 hPrintf("<div class=\"inputGroup\">");
@@ -2300,56 +2303,62 @@
     freez(&errMsg);
     errMsg = cloneString("Could not start social login. Please try again.");
     displayLoginPage(conn);
     return;
     }
 jsInlineF("window.location = '%s';\n", url);
 }
 
 void oauthReturn(struct sqlConnection *conn)
 /* Handle the provider's redirect back to us: verify state, exchange the code for the
  * user's identity, and resolve/auto-link the account. */
 {
 char *state = cgiUsualString("state", "");
 char *savedState = cartUsualString(cart, "oauth_state", "");
 char *provider = cartUsualString(cart, "oauth_provider", "");
+
+// Validate the anti-CSRF state before consuming any cart state or acting on an error param.  A
+// stray code/error link (a re-opened redirect, or a crafted hgLogin?error=...) must not be able to
+// consume the state nonce of a login in flight, so check first and only then clear the flow.  A
+// compliant provider echoes state on an error return too (RFC 6749 4.1.2.1), and we always send it.
+if (isEmpty(state) || isEmpty(savedState) || differentString(state, savedState))
+    {
+    freez(&errMsg);
+    errMsg = cloneString("Your login session expired or was invalid. Please try again.");
+    displayLoginPage(conn);
+    return;
+    }
 cartRemove(cart, "oauth_state");      // one-time use
+cartRemove(cart, "oauth_provider");   // end the flow so a later code/error param can't re-enter
 
 char *errParam = cgiUsualString("error", "");
 if (isNotEmpty(errParam))
     {
     // The provider redirected back with an OAuth error instead of a code (e.g. the user
     // declined, or the client is misconfigured/unapproved).  Show its message rather than
     // silently falling through to another page.
     char *desc = cgiUsualString("error_description", "");
     struct dyString *dy = dyStringNew(256);
     dyStringAppend(dy, "Social login failed. ");
     if (isNotEmpty(desc))
         dyStringPrintf(dy, "%s ", htmlEncode(desc));
     dyStringPrintf(dy, "(%s)", htmlEncode(errParam));
     freez(&errMsg);
     errMsg = dyStringCannibalize(&dy);
     displayLoginPage(conn);
     return;
     }
-if (isEmpty(state) || isEmpty(savedState) || differentString(state, savedState))
-    {
-    freez(&errMsg);
-    errMsg = cloneString("Your login session expired or was invalid. Please try again.");
-    displayLoginPage(conn);
-    return;
-    }
 char *code = cgiUsualString("code", "");
 struct oauthIdentity *id = oauthFetchIdentity(provider, code, hgLoginUrl);
 if (id == NULL)
     {
     freez(&errMsg);
     errMsg = cloneString("We could not complete the social login. Please try again.");
     displayLoginPage(conn);
     return;
     }
 resolveIdentity(conn, id);
 oauthIdentityFree(&id);
 }
 
 void emailLinkPage(struct sqlConnection *conn)
 /* Standalone page that asks for an email address and sends a one-time login link. */