Merge pull request #587 from seznam/usa-3554

Cookie security

.changeset/brown-rivers-confess.md

@@ -0,0 +1,5 @@
+---
+"@ima/core": patch
+---
+
+Fixed cookie parsing from setCookie header when multiple cookies were sent to server. Previously only the first cookie was parsed while multiple set-cookies could alter the cookie settings

.changeset/flat-beds-travel.md

@@ -0,0 +1,6 @@
+---
+"create-ima-app": minor
+"@ima/core": minor
+---
+
+Added new settings `validateCookies` to enable/disable cookie validation. It validates cookie options and request url before saving cookie or sending it to the server. This means that path, subdomain and secure options must match between the request url and the cookie, otherwise the cookie is not saved or sent.

packages/core/src/http/HttpAgent.ts

@@ -39,6 +39,7 @@ export interface HttpAgentRequestOptions {
   ) => HttpAgentResponse<B>)[];
   abortController?: AbortController;
   keepSensitiveHeaders?: boolean;
+  validateCookies?: boolean;
 }
 
 /**

packages/core/src/http/HttpAgentImpl.ts

@@ -246,7 +246,7 @@ export class HttpAgentImpl extends HttpAgent {
     data?: UnknownParameters,
     options?: Partial<HttpAgentRequestOptions>
   ): Promise<HttpAgentResponse<B>> {
-    const optionsWithDefault = this._prepareOptions(options);
+    const optionsWithDefault = this._prepareOptions(options, url);
 
     if (optionsWithDefault.cache) {
       const cachedData = this._getCachedData<B>(method, url, data);
@@ -433,7 +433,8 @@ export class HttpAgentImpl extends HttpAgent {
    *         internally.
    */
   _prepareOptions(
-    options: Partial<HttpAgentRequestOptions> = {}
+    options: Partial<HttpAgentRequestOptions> = {},
+    url: string
   ): HttpAgentRequestOptions {
     const composedOptions = {
       ...this._defaultRequestOptions,
@@ -455,7 +456,9 @@ export class HttpAgentImpl extends HttpAgent {
     if (composedOptions.fetchOptions?.credentials === 'include') {
       // mock default browser behavior for server-side (sending cookie with a fetch request)
       composedOptions.fetchOptions.headers.Cookie =
-        this._cookie.getCookiesStringForCookieHeader();
+        this._cookie.getCookiesStringForCookieHeader(
+          options.validateCookies ? url : undefined
+        );
     }
 
     return composedOptions;
@@ -497,10 +500,15 @@ export class HttpAgentImpl extends HttpAgent {
    */
   _setCookiesFromResponse<B>(agentResponse: HttpAgentResponse<B>): void {
     if (agentResponse.headersRaw) {
-      const receivedCookies = agentResponse.headersRaw.get('set-cookie');
-
-      if (receivedCookies) {
-        this._cookie.parseFromSetCookieHeader(receivedCookies);
+      const receivedCookies = agentResponse.headersRaw.getSetCookie();
+
+      if (receivedCookies.length > 0) {
+        this._cookie.parseFromSetCookieHeader(
+          receivedCookies,
+          this._defaultRequestOptions.validateCookies
+            ? agentResponse.params.url
+            : undefined
+        );
       }
     }
   }

packages/core/src/http/__tests__/HttpAgentImplSpec.ts

@@ -71,11 +71,9 @@ describe('ima.core.http.HttpAgentImpl', () => {
         'set-cookie': ['cookie1=cookie1', 'cookie2=cookie2'],
       },
       // @ts-ignore
-      headersRaw: new Map(
-        Object.entries({
-          'set-cookie': ['cookie1=cookie1', 'cookie2=cookie2'],
-        })
-      ),
+      headersRaw: new Headers({
+        'set-cookie': ['cookie1=cookie1', 'cookie2=cookie2'],
+      }),
     };
   });
 

packages/core/src/storage/CookieStorage.ts

@@ -96,6 +96,77 @@ export class CookieStorage extends Storage<Cookie['value']> {
     return [Window, Request, Response];
   }
 
+  /**
+   * Filters invalid cookies based on the provided url.
+   * We try to check validity of the domain based on secure, path and
+   * domain definitions.
+   */
+  static validateCookieSecurity(cookie: Cookie, url: string): boolean {
+    const { pathname, hostname, protocol } = new URL(url);
+    const secure = protocol === 'https:';
+
+    /**
+     * Don't allow setting secure cookies without the secure
+     * defined in the validate options.
+     */
+    if (
+      typeof cookie.options.secure === 'boolean' &&
+      secure !== cookie.options.secure
+    ) {
+      return false;
+    }
+
+    /**
+     * Don't allow setting cookies with a path that doesn't start with
+     * the path defined in the validate options. Root path is always valid.
+     */
+    if (
+      typeof cookie.options.path === 'string' &&
+      cookie.options.path !== '/'
+    ) {
+      const pathChunks = pathname.split('/');
+      const cookiePathChunks = cookie.options.path.split('/');
+
+      /**
+       * Compare the path chunks of the request path and the cookie path.
+       */
+      for (let i = 0; i < cookiePathChunks.length; i++) {
+        /**
+         * There are no more path chunks to compare, so the cookie path is a
+         * prefix of the request path.
+         */
+        if (cookiePathChunks[i] === undefined) {
+          break;
+        }
+
+        /**
+         * The path chunks don't match, the cookie path is not a prefix of
+         * the request path.
+         */
+        if (pathChunks[i] !== cookiePathChunks[i]) {
+          return false;
+        }
+      }
+    }
+
+    /**
+     * Domain security check, we also check for subdomain match.
+     */
+    if (cookie.options.domain) {
+      const cookieDomain = cookie.options.domain.toLowerCase();
+      const normalizedCookieDomain = cookieDomain.startsWith('.')
+        ? cookieDomain.slice(1)
+        : cookieDomain;
+
+      return normalizedCookieDomain === hostname ||
+        hostname.endsWith(normalizedCookieDomain)
+        ? true
+        : false;
+    }
+
+    return true;
+  }
+
   /**
    * Initializes the cookie storage.
    *
@@ -238,15 +309,29 @@ export class CookieStorage extends Storage<Cookie['value']> {
    * Returns all cookies in this storage serialized to a string compatible
    * with the `Cookie` HTTP header.
    *
+   * When `url` is provided, the method validates the cookie security based on
+   * the `url` and the cookie's domain, path, and secure attributes.
+   *
    * @return All cookies in this storage serialized to a string
    *         compatible with the `Cookie` HTTP header.
    */
-  getCookiesStringForCookieHeader(): string {
+  getCookiesStringForCookieHeader(url?: string): string {
     const cookieStrings = [];
 
     for (const cookieName of this._storage.keys()) {
       const cookieItem = this._storage.get(cookieName);
 
+      /**
+       * Skip cookies that are not secure for the provided url.
+       */
+      if (
+        url &&
+        cookieItem &&
+        !CookieStorage.validateCookieSecurity(cookieItem, url)
+      ) {
+        continue;
+      }
+
       cookieStrings.push(
         this.#generateCookieString(cookieName, cookieItem!.value, {})
       );
@@ -258,18 +343,42 @@ export class CookieStorage extends Storage<Cookie['value']> {
   /**
    * Parses cookies from the provided `Set-Cookie` HTTP header value.
    *
+   * When `url` is provided, the method validates the cookie security based on
+   * the `url` and the cookie's domain, path, and secure attributes.
+   *
    * The parsed cookies will be set to the internal storage, and the current
    * HTTP response (via the `Set-Cookie` HTTP header) if at the server
    * side, or the browser (via the `document.cookie` property).
    *
-   * @param setCookieHeader The value of the `Set-Cookie` HTTP
-   *        header.
+   * @param cookiesString The value of the `Set-Cookie` HTTP
+   *        header. When there are multiple cookies, the value can be
+   *        provided as an array of strings.
    */
-  parseFromSetCookieHeader(setCookieHeader: string): void {
-    const cookie = this.#extractCookie(setCookieHeader);
+  parseFromSetCookieHeader(
+    cookiesString: string | string[],
+    url?: string
+  ): void {
+    const cookiesArray = Array.isArray(cookiesString)
+      ? cookiesString
+      : [cookiesString];
+
+    for (const cookie of cookiesArray) {
+      const cookieItem = this.#extractCookie(cookie);
+
+      /**
+       * Skip cookies that are not secure for the provided url.
+       */
+      if (
+        url &&
+        cookieItem &&
+        !CookieStorage.validateCookieSecurity(cookieItem, url)
+      ) {
+        continue;
+      }
 
-    if (typeof cookie.name === 'string') {
-      this.set(cookie.name, cookie.value, cookie.options);
+      if (typeof cookieItem.name === 'string') {
+        this.set(cookieItem.name, cookieItem.value, cookieItem.options);
+      }
     }
   }