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/inc/https.h src/inc/https.h index 054f7a3c947..bf84ba67449 100644 --- src/inc/https.h +++ src/inc/https.h @@ -1,9 +1,15 @@ /* Connect via https. */ #ifndef NET_HTTPS_H #define NET_HTTPS_H int netConnectHttps(char *hostName, int port, boolean noProxy, char *httpProtocol); /* Return socket for https connection with server or -1 if error. */ +void httpsSetCertCheck(char *mode); +/* Pin the TLS certificate-check mode ("abort", "warn", or "log") for every HTTPS connection this + * process makes from here on, overriding hg.conf's httpsCertCheck and the https_cert_check env + * var. Use where a caller must never accept an unverified certificate however the site is + * configured. Order-independent: safe to call before or after the first HTTPS connection. */ + #endif//ndef NET_HTTPS_H