8f329215737bf098902eb0d589be5c1b29008aa4 braney Thu Aug 13 08:18:44 2026 -0700 hgLogin: treat the login-flow cart variables as server-owned, refs #38037 diff --git src/hg/hgLogin/hgLogin.c src/hg/hgLogin/hgLogin.c index 50993c7fd99..7009e166211 100644 --- src/hg/hgLogin/hgLogin.c +++ src/hg/hgLogin/hgLogin.c @@ -2507,53 +2507,77 @@ "UPDATE gbMembers SET loginToken='', lastUse=NOW() WHERE idx=%u", list->idx); sqlUpdate(conn, query); loginAndReturn(list->userName, list->idx); } else { /* Several accounts share this now-verified address: stash the proof and let the user * pick one. chooseAccount re-checks the token before logging in. */ cartSetString(cart, "emailLogin_email", email); cartSetString(cart, "emailLogin_tokenMd5", tokenMD5); chooseAccountPage(conn); } gbMembersFreeList(&list); } +static void dropRequestSuppliedFlowVars() +/* The cart variables holding the state of a login in flight are written by hgLogin and by + * nothing else: the nonce and provider of an OAuth round trip, the pending identity behind + * the account chooser, and the verified address behind the email-link chooser. The cart + * takes CGI variables verbatim (loadCgiOverHash in hg/lib/cart.c), so a copy arriving with + * the request would stand in for the copy we stored. Drop those before anything reads them; + * a flow whose state is dropped fails closed and the user starts it again. Note that + * excludeVars would not do this job: it governs what is saved at the end of a request, not + * what is read during it. */ +{ +static char *serverOwned[] = { + "oauth_state", "oauth_provider", + "oauth_pending_provider", "oauth_pending_subject", "oauth_pending_email", + "oauth_pending_email_verified", "oauth_pending_name", "oauth_pending_time", + "oauth_pending_sig", + "emailLogin_email", "emailLogin_tokenMd5", + }; +int i; +for (i = 0; i < ArraySize(serverOwned); i++) + if (cgiVarExists(serverOwned[i])) + cartRemove(cart, serverOwned[i]); +} + void doMiddle(struct cart *theCart) /* Write the middle parts of the HTML page. * This routine sets up some globals and then * dispatches to the appropriate page-maker. */ { struct sqlConnection *conn = hConnectCentral(); // on mirrors, try to add the field 'recovEmail' to gbMembers. This may or may not work, depending on their config if (sqlFieldIndex(conn, "gbMembers", "recovEmail") == -1) { autoUpgradeTableAddColumn(conn, "gbMembers", "recovEmail", "varchar(255)", FALSE, "''"); } // columns for the passwordless email-link login feature if (sqlFieldIndex(conn, "gbMembers", "loginToken") == -1) autoUpgradeTableAddColumn(conn, "gbMembers", "loginToken", "varchar(255)", FALSE, "NULL"); if (sqlFieldIndex(conn, "gbMembers", "loginTokenExpires") == -1) autoUpgradeTableAddColumn(conn, "gbMembers", "loginTokenExpires", "DATETIME", FALSE, "NULL"); // table linking accounts to social (Google/ORCID) identities; only needed where OAuth is set up if (oauthAnyProviderEnabled()) createIdentityTable(conn); cart = theCart; +dropRequestSuppliedFlowVars(); safecpy(brwName,sizeof(brwName), browserName()); safecpy(brwAddr,sizeof(brwAddr), browserAddr()); safecpy(signature,sizeof(signature), mailSignature()); safecpy(returnAddr,sizeof(returnAddr), mailReturnAddr()); pwdEyeIconEnabled = cfgOptionBooleanDefault(CFG_LOGIN_PWD_EYE_ICON, TRUE); // A provider's OAuth redirect back to us carries 'code' (success) or 'error' (failure) but // none of our own hgLogin.do.* variables, so detect it up front. We gate on an OAuth flow // being in progress (oauth_provider set by oauthStart) so a stray code/error param can't // trigger this. Error returns may omit 'code' and even 'state', so we must not require them. if ((cgiOptionalString("code") != NULL || cgiOptionalString("error") != NULL) && isNotEmpty(cartUsualString(cart, "oauth_provider", ""))) oauthReturn(conn); else if (cartVarExists(cart, "hgLogin.do.oauthStart")) oauthStart(conn);