Validate the OAuth state parameter on the web-flow callback

src/renderer/utils/forges/github/flows.test.ts

@@ -150,11 +150,12 @@ describe('renderer/utils/forges/github/flows.ts', () => {
     };
 
     it('should call performGitHubWebOAuth using custom oauth app - success oauth flow', async () => {
+      let deliverCallback: (callbackUrl: string) => void = () => {};
       window.gitify.onAuthCallback = vi.fn().mockImplementation((callback) => {
-        callback('gitify://oauth?code=123-456');
+        deliverCallback = callback;
       });
 
-      const res = await flows.performGitHubWebOAuth({
+      const resultPromise = flows.performGitHubWebOAuth({
         clientId: 'BYO_CLIENT_ID' as ClientID,
         clientSecret: 'BYO_CLIENT_SECRET' as ClientSecret,
         hostname: 'my.git.com' as Hostname,
@@ -167,13 +168,55 @@ describe('renderer/utils/forges/github/flows.ts', () => {
         ),
       );
 
+      // The callback must echo back the same `state` this attempt generated;
+      // read it from the authorization URL just asserted above.
+      const authorizeUrl = new URL(openExternalLinkSpy.mock.calls[0][0] as string);
+      const state = authorizeUrl.searchParams.get('state');
+      deliverCallback(`gitify://oauth?code=123-456&state=${state}`);
+
+      const res = await resultPromise;
+
       expect(window.gitify.onAuthCallback).toHaveBeenCalledTimes(1);
       expect(window.gitify.onAuthCallback).toHaveBeenCalledWith(expect.any(Function));
 
       expect(res.authMethod).toBe('OAuth App');
       expect(res.authCode).toBe('123-456');
     });
 
+    it('does not resolve when the callback state does not match the generated state', async () => {
+      let deliverCallback: (callbackUrl: string) => void = () => {};
+      window.gitify.onAuthCallback = vi.fn().mockImplementation((callback) => {
+        deliverCallback = callback;
+      });
+
+      let resolved = false;
+      const resultPromise = flows
+        .performGitHubWebOAuth(webAuthOptions)
+        .then((res) => {
+          resolved = true;
+          return res;
+        });
+
+      deliverCallback('gitify://oauth?code=attacker-code&state=not-the-real-state');
+      deliverCallback('gitify://oauth?code=attacker-code-no-state');
+
+      // Give any pending microtasks a chance to run, then confirm nothing
+      // resolved the promise from either mismatched callback.
+      await Promise.resolve();
+      await Promise.resolve();
+      expect(resolved).toBe(false);
+
+      // Deliver the real callback so the promise settles and the test
+      // doesn't leave a dangling unhandled rejection.
+      const authorizeUrl = new URL(
+        openExternalLinkSpy.mock.calls[openExternalLinkSpy.mock.calls.length - 1][0] as string,
+      );
+      const state = authorizeUrl.searchParams.get('state');
+      deliverCallback(`gitify://oauth?code=the-real-code&state=${state}`);
+      const res = await resultPromise;
+      expect(res.authCode).toBe('the-real-code');
+    });
+
     it('should call performGitHubWebOAuth - failure', async () => {
       window.gitify.onAuthCallback = vi.fn().mockImplementation((callback) => {
         callback(

src/renderer/utils/forges/github/flows.ts

@@ -23,6 +23,17 @@ type DeviceFlowErrorResponse = {
   error_uri: string;
 };
 
+/**
+ * Generates a cryptographically random OAuth `state` value for one web-flow
+ * login attempt, using `crypto.getRandomValues` per this codebase's
+ * secure-coding convention (see `utils/core/random.ts`).
+ */
+function generateOAuthState(): string {
+  const buf = new Uint8Array(16);
+  crypto.getRandomValues(buf);
+  return Array.from(buf, (b) => b.toString(16).padStart(2, '0')).join('');
+}
+
 /**
  * Initiate a GitHub OAuth Web Flow (OAuth App) authentication.
  *
@@ -34,11 +45,19 @@ type DeviceFlowErrorResponse = {
  */
 export function performGitHubWebOAuth(authOptions: LoginOAuthWebOptions): Promise<AuthResponse> {
   return new Promise((resolve, reject) => {
+    // A per-attempt, unguessable value this app itself tracks and checks on
+    // callback, so a `gitify://oauth` callback carrying an authorization code
+    // this specific login attempt never requested (e.g. one an attacker
+    // obtained for their own GitHub account) is rejected instead of silently
+    // accepted and exchanged for a token.
+    const expectedState = generateOAuthState();
+
     const { url } = getWebFlowAuthorizationUrl({
       clientType: 'oauth-app',
       clientId: authOptions.clientId,
       scopes: getRecommendedScopeNames(),
       allowSignup: false,
+      state: expectedState,
       request: request.defaults({
         baseUrl: `https://${authOptions.hostname}`,
       }),
@@ -63,17 +82,28 @@ export function performGitHubWebOAuth(authOptions: LoginOAuthWebOptions): Promis
 
       const type = url.hostname;
       const code = url.searchParams.get('code');
+      const state = url.searchParams.get('state');
       const error = url.searchParams.get('error');
       const errorDescription = url.searchParams.get('error_description');
       const errorUri = url.searchParams.get('error_uri');
 
-      if (code && type === 'oauth') {
+      if (code && type === 'oauth' && state === expectedState) {
         unsubscribeAuthCallback?.();
         resolve({
           authMethod: 'OAuth App',
           authCode: code as AuthCode,
           authOptions: authOptions,
         });
+      } else if (code && type === 'oauth') {
+        // A code arrived but its state doesn't match this login attempt's
+        // own value (or carries none) - do not treat it as this login
+        // completing, since accepting it would let a code obtained outside
+        // this flow (e.g. for a different GitHub account) be exchanged for
+        // a token as if it belonged to this session.
+        rendererLogError(
+          'renderer:auth-callback',
+          'Rejected an authentication callback with a missing or mismatched state parameter',
+        );
       } else if (error) {
         unsubscribeAuthCallback?.();
         reject(