CVE-2022-0087: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: keystone is vulnerable to Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 6.1
Published: 2022-01-12T00:15Z
Repository: https://github.com/keystonejs/keystone
Language: Other
Files changed: .changeset/quiet-wasps-draw.md, packages/auth/src/index.ts, packages/auth/src/lib/useFromRedirect.ts, packages/auth/src/pages/InitPage.tsx, packages/auth/src/pages/SigninPage.tsx
Commit date: 2022-01-10 13:17:09 +1100
Commit message: Do not add `from` parameter to URL on signin page when it matches the default redirect

Additionally extract redirect logic for ease of reuse

Diff: diff --git a/.changeset/quiet-wasps-draw.md b/.changeset/quiet-wasps-draw.md
new file mode 100644
index 000000000..07b3edfbf
--- /dev/null
+++ b/.changeset/quiet-wasps-draw.md
@@ -0,0 +1,6 @@
+---
+'@keystone-6/auth': patch
+---
+
+When redirecting to the homepage,  the `from` query parameter unnecessarily displayed `=/`.
+This patch fixes that so it is only provided as a query parameter when needed.
diff --git a/packages/auth/src/index.ts b/packages/auth/src/index.ts
index c9bd28cc3..f2c9b4b36 100644
--- a/packages/auth/src/index.ts
+++ b/packages/auth/src/index.ts
@@ -112,11 +112,12 @@ export function createAuth<ListTypeInfo extends BaseListTypeInfo>({
         return;
       }
     }
 
     if (!session && pathname !== '/signin') {
-      return { kind: 'redirect', to: `/signin?from=${encodeURIComponent(req!.url!)}` };
+      let to = pathname === '/' ? '/signin' : `/signin?from=${encodeURIComponent(req!.url!)}`;
+      return { kind: 'redirect', to };
     }
   };
 
   /**
    * getAdditionalFiles
diff --git a/packages/auth/src/lib/useFromRedirect.ts b/packages/auth/src/lib/useFromRedirect.ts
new file mode 100644
index 000000000..2828d52db
--- /dev/null
+++ b/packages/auth/src/lib/useFromRedirect.ts
@@ -0,0 +1,15 @@
+import { useMemo } from 'react';
+import { useRouter } from '@keystone-6/core/admin-ui/router';
+
+export const useRedirect = () => {
+  const router = useRouter();
+  const redirect = useMemo(
+    () =>
+      !Array.isArray(router.query.from) && router.query.from?.startsWith('/')
+        ? router.query.from
+        : '/',
+    [router]
+  );
+
+  return redirect;
+};
diff --git a/packages/auth/src/pages/InitPage.tsx b/packages/auth/src/pages/InitPage.tsx
index f26d6f0aa..a160f1ed9 100644
--- a/packages/auth/src/pages/InitPage.tsx
+++ b/packages/auth/src/pages/InitPage.tsx
@@ -22,19 +22,22 @@ import {
 } from '@keystone-6/core/admin-ui/utils';
 import { LoadingDots } from '@keystone-ui/loading';
 import { guessEmailFromValue, validEmail } from '../lib/emailHeuristics';
 import { IconTwitter, IconGithub } from '../components/Icons';
 import { SigninContainer } from '../components/SigninContainer';
+import { useRedirect } from '../lib/useFromRedirect';
 
 const signupURL = 'https://signup.keystonejs.cloud/api/newsletter-signup';
 
 const Welcome = ({ value }: { value: any }) => {
   const [subscribe, setSubscribe] = useState<boolean>(false);
   const [email, setEmail] = useState<string>(guessEmailFromValue(value));
   const [error, setError] = useState<string | null>(null);
   const [loading, setLoading] = useState(false);
   const router = useRouter();
+  const redirect = useRedirect();
+
   const onSubmit = (event: React.FormEvent) => {
     event.preventDefault();
     setError(null);
     // Check if user wants to subscribe.
     // and there's a valid email address.
@@ -63,11 +66,11 @@ const Welcome = ({ value }: { value: any }) => {
               res.json().then(({ error }) => {
                 setError(error);
                 setLoading(false);
               });
             } else {
-              router.push((router.query.from as string | undefined) || '/');
+              router.push(redirect);
             }
           })
           .catch(err => {
             // network errors or failed parse
             setError(err.toString());
@@ -78,11 +81,11 @@ const Welcome = ({ value }: { value: any }) => {
         // if email fails validation set error message
         setError('Email is invalid');
         return;
       }
     }
-    return router.push((router.query.from as string | undefined) || '/');
+    return router.push(redirect);
   };
   return (
     <Stack gap="large">
       <Stack
         gap="small"
@@ -193,10 +196,11 @@ const InitPage = ({ fieldPaths, listKey, enableWelcome }: InitPageProps) => {
       }
     }
   }`);
   const reinitContext = useReinitContext();
   const router = useRouter();
+  const redirect = useRedirect();
   const rawKeystone = useRawKeystone();
 
   useEffect(() => {
     // This effect handles both cases:
     // a ) Our form submission is complete with new data
@@ -208,14 +212,14 @@ const InitPage = ({ fieldPaths, listKey, enableWelcome }: InitPageProps) => {
       // To tell the component to render the Welcome screen
       if (enableWelcome) {
         setMode('welcome');
       } else {
         // otherwise we route them through to the admin dashboard
-        router.push((router.query.from as string | undefined) || '/');
+        router.push(redirect);
       }
     }
-  }, [rawKeystone.authenticatedItem, enableWelcome, router]);
+  }, [rawKeystone.authenticatedItem, enableWelcome, router, redirect]);
 
   if (rawKeystone.authenticatedItem.state === 'authenticated' && !enableWelcome) {
     return (
       <Center fillView>
         <LoadingDots label="Loading page" size="large" />
diff --git a/packages/auth/src/pages/SigninPage.tsx b/packages/auth/src/pages/SigninPage.tsx
index c12873527..da6cd68c5 100644
--- a/packages/auth/src/pages/SigninPage.tsx
+++ b/packages/auth/src/pages/SigninPage.tsx
@@ -1,9 +1,9 @@
 /** @jsxRuntime classic */
 /** @jsx jsx */
 
-import { useState, Fragment, FormEvent, useRef, useEffect } from 'react';
+import { useState, Fragment, FormEvent, useRef, useEffect, useMemo } from 'react';
 
 import { jsx, H1, Stack, VisuallyHidden, Center } from '@keystone-ui/core';
 import { Button } from '@keystone-ui/button';
 import { TextInput } from '@keystone-ui/fields';
 import { Notice } from '@keystone-ui/notice';
@@ -11,10 +11,11 @@ import { Notice } from '@keystone-ui/notice';
 import { useMutation, gql } from '@keystone-6/core/admin-ui/apollo';
 import { useRawKeystone, useReinitContext } from '@keystone-6/core/admin-ui/context';
 import { useRouter } from '@keystone-6/core/admin-ui/router';
 import { LoadingDots } from '@keystone-ui/loading';
 import { SigninContainer } from '../components/SigninContainer';
+import { useRedirect } from '../lib/useFromRedirect';
 
 type SigninPageProps = {
   identityField: string;
   secretField: string;
   mutationName: string;
@@ -56,16 +57,18 @@ export const SigninPage = ({
 
   const [mutate, { error, loading, data }] = useMutation(mutation);
   const reinitContext = useReinitContext();
   const router = useRouter();
   const rawKeystone = useRawKeystone();
+  const redirect = useRedirect();
 
+  // This useEffect specifically handles ending up on the signin page from a SPA navigation
   useEffect(() => {
     if (rawKeystone.authenticatedItem.state === 'authenticated') {
-      router.push((router.query.from as string | undefined) || '/');
+      router.push(redirect);
     }
-  }, [rawKeystone.authenticatedItem, router]);
+  }, [rawKeystone.authenticatedItem, router, redirect]);
 
   if (rawKeystone.authenticatedItem.state === 'authenticated') {
     return (
       <Center fillView>
         <LoadingDots label="Loading page" size="large" />
@@ -94,11 +97,11 @@ export const SigninPage = ({
               }
             } catch (err) {
               return;
             }
             reinitContext();
-            router.push((router.query.from as string | undefined) || '/');
+            router.push(redirect);
           }
         }}
       >
         <H1>Sign In</H1>
         {error && (

Vulnerable code:       return { kind: 'redirect', to: `/signin?from=${encodeURIComponent(req!.url!)}` };
              router.push((router.query.from as string | undefined) || '/');
    return router.push((router.query.from as string | undefined) || '/');
        router.push((router.query.from as string | undefined) || '/');
  }, [rawKeystone.authenticatedItem, enableWelcome, router]);
import { useState, Fragment, FormEvent, useRef, useEffect } from 'react';
      router.push((router.query.from as string | undefined) || '/');
  }, [rawKeystone.authenticatedItem, router]);
            router.push((router.query.from as string | undefined) || '/');
Fixed code: ---
'@keystone-6/auth': patch
---

When redirecting to the homepage,  the `from` query parameter unnecessarily displayed `=/`.
This patch fixes that so it is only provided as a query parameter when needed.
      let to = pathname === '/' ? '/signin' : `/signin?from=${encodeURIComponent(req!.url!)}`;
      return { kind: 'redirect', to };
import { useMemo } from 'react';
import { useRouter } from '@keystone-6/core/admin-ui/router';

export const useRedirect = () => {
  const router = useRouter();
  const redirect = useMemo(
    () =>
      !Array.isArray(router.query.from) && router.query.from?.startsWith('/')
        ? router.query.from
        : '/',
    [router]
  );

  return redirect;
};
import { useRedirect } from '../lib/useFromRedirect';
  const redirect = useRedirect();

              router.push(redirect);
    return router.push(redirect);
  const redirect = useRedirect();
        router.push(redirect);
  }, [rawKeystone.authenticatedItem, enableWelcome, router, redirect]);
import { useState, Fragment, FormEvent, useRef, useEffect, useMemo } from 'react';
import { useRedirect } from '../lib/useFromRedirect';
  const redirect = useRedirect();
  // This useEffect specifically handles ending up on the signin page from a SPA navigation
      router.push(redirect);
  }, [rawKeystone.authenticatedItem, router, redirect]);
            router.push(redirect);