e81efb1074d9786436ed8cdb626fc3bdeb6ac14c
max
  Thu Aug 6 09:20:30 2026 -0700
hgLogin: fix the social-login/email-link code review issues from #38008

#Preview2 week - bugs introduced now will need a build patch to fix
Brian, thanks for the thorough review - every one of these was real. Here is what
I changed for the six items that stayed on the ticket (the pre-existing XSS and the
site-wide httpsCertCheck default went to #38011 and #38012).

1. Reflected XSS in the account chooser and the other new pages. Every address and
username now goes through htmlEncode() before it lands in the HTML or an attribute
(chooseAccountPage, completeAccountPage, emailLinkPage, changeEmailPage and the
confirmation pages). I also gated the email-link side of the chooser on
emailLinkEnabled(), so your chooseAccount + emailLogin_email=<img ...> URL now
renders the tag as text and does nothing at all where the feature is off.

2. Registering someone else's address to steal their social login. The two OAuth
email-match queries (resolveIdentity and chooseAccount) now require
accountActivated='Y', so an unactivated row someone planted with a victim's address
can no longer be matched or linked. completeAccount only marks the new account
activated when the provider actually verified the address and the user kept it;
otherwise it creates the account inactive and sends the usual confirmation mail, so
an unverified address can never be planted as a trusted one.

3. OAuth requests not enforcing the server certificate. Rather than poke the env var,
I added a small library knob, httpsSetCertCheck() in lib/https.c, that pins the
cert-check mode for the rest of the process and is not overwritten by openSslInit()
or hg.conf. hgLogin's httpRequest() calls it with "abort", so those requests refuse
a bad certificate no matter how the site is configured, and it no longer depends on
being the first HTTPS connection.

4. The pending-identity signature. It now also covers the hguid (which survives the
provider redirect, unlike the hgsid) and the time it was minted, with a 15-minute
expiry, and it is cleared on the failure paths too. A signature that leaks into a
saved or shared session is now useless to another browser and dies quickly anyway.

5. changeEmail. It now asks for the current password where the account has one, and it
no longer changes the address on the spot - it emails a one-time signed confirmation
link to the new address and only applies the change when that link is opened. When the
change lands it also mails the OLD address to say the account's email was changed and
who to contact if that wasn't them, so a hijack gets noticed. One honest caveat: an
account with no password (social-only) still can't be re-checked before the change, so
a stolen cookie could still start it - but the old-address alert now gives the owner a
way to catch it. Expiring login cookies is the deeper fix and feels like its own ticket.

6. isalnum() on a signed char in suggestUsername - now cast to unsigned char.

Build is clean, no new warnings. Set back to you for another look.

refs #38008

diff --git src/hg/hgLogin/oauthLogin.c src/hg/hgLogin/oauthLogin.c
index 811ad168121..c554f4f9a2d 100644
--- src/hg/hgLogin/oauthLogin.c
+++ src/hg/hgLogin/oauthLogin.c
@@ -1,27 +1,28 @@
 /* oauthLogin - social login for hgLogin via OAuth 2.0 / OpenID Connect.
  * See oauthLogin.h for the hg.conf configuration. */
 
 /* Copyright (C) 2026 The Regents of the University of California
  * See kent/LICENSE or http://genome.ucsc.edu/license/ for licensing information. */
 
 #include "common.h"
 #include "cheapcgi.h"
 #include "hgConfig.h"
 #include "dystring.h"
 #include "errCatch.h"
 #include "net.h"
+#include "https.h"
 #include "htmlPage.h"
 #include "jsonParse.h"
 #include "oauthLogin.h"
 
 struct oauthProvider
 /* One configured social login provider, built from hg.conf. */
     {
     struct oauthProvider *next;
     char *name;             /* short key, used in URLs and gbMemberIdentity.provider */
     char *label;            /* button label */
     char *type;             /* "oidc" or "github" */
     char *clientId;
     char *clientSecret;
     char *authUrl;          /* authorization endpoint */
     char *tokenUrl;         /* token endpoint */
@@ -198,30 +199,38 @@
 return names;
 }
 
 char *oauthProviderLabel(char *name)
 /* Return the display label for a provider (falls back to the name). */
 {
 struct oauthProvider *p = providerByName(name);
 return (p != NULL) ? p->label : name;
 }
 
 /* ---- HTTP helpers ---- */
 
 static char *httpRequest(char *url, char *method, char *header, char *body)
 /* Make an HTTP(S) request and return the response body (allocd), or NULL on failure. */
 {
+/* Insist on a valid TLS certificate for these calls, whatever the site's httpsCertCheck is set
+ * to.  We send the client secret to the token endpoint and then trust the identity in the
+ * response, so a forged certificate on a man-in-the-middle would hand over the secret and let an
+ * attacker claim any verified email.  The default httpsCertCheck is "log", which accepts any
+ * certificate.  httpsSetCertCheck() pins "abort" for the rest of this process regardless of when
+ * the first HTTPS connection happens; this is the choke point every outbound request goes
+ * through, so pinning it here covers all of them. */
+httpsSetCertCheck("abort");
 char *result = NULL;
 struct errCatch *errCatch = errCatchNew();
 if (errCatchStart(errCatch))
     {
     int sd = netOpenHttpExt(url, method, header);
     if (sd >= 0)
         {
         if (isNotEmpty(body))
             mustWriteFd(sd, body, strlen(body));
         struct dyString *dy = netSlurpFile(sd);
         close(sd);
         struct htmlPage *page = htmlPageParse(url, dyStringCannibalize(&dy));
         if (page != NULL && isNotEmpty(page->htmlText))
             result = cloneString(page->htmlText);
         }