Merge 40b3289 into 4b44553

firebase · Feb 10, 2023 · 5640893 · 5640893
2 parents 4b44553 + 40b3289
commit 5640893
Show file tree

Hide file tree

Showing 8 changed files with 88 additions and 12 deletions.
diff --git a/.changeset/popular-apples-peel.md b/.changeset/popular-apples-peel.md
@@ -0,0 +1,7 @@
+---
+'@firebase/firestore': patch
+'@firebase/util': patch
+'firebase': patch
+---
+
+Modify base64 decoding logic to throw on invalid input, rather than silently truncating it.
diff --git a/common/api-review/util.api.md b/common/api-review/util.api.md
@@ -93,6 +93,14 @@ export function createSubscribe<T>(executor: Executor<T>, onNoObservers?: Execut
 // @public
 export const decode: (token: string) => DecodedToken;
 
+// Warning: (ae-missing-release-tag) "DecodeBase64StringError" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
+//
+// @public
+export class DecodeBase64StringError extends Error {
+    // (undocumented)
+    readonly name = "DecodeBase64StringError";
+}
+
 // Warning: (ae-missing-release-tag) "deepCopy" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
 //
 // @public

diff --git a/packages/firestore/src/platform/base64.ts b/packages/firestore/src/platform/base64.ts
@@ -15,13 +15,24 @@
  * limitations under the License.
  */
 
+import { Base64DecodeError } from '../util/base64_decode_error';
+
 // This file is only used under ts-node.
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 const platform = require(`./${process.env.TEST_PLATFORM ?? 'node'}/base64`);
 
 /** Converts a Base64 encoded string to a binary string. */
 export function decodeBase64(encoded: string): string {
-  return platform.decodeBase64(encoded);
+  const decoded = platform.decodeBase64(encoded);
+
+  // A quick sanity check as node and rn will not throw error if input is an
+  // invalid base64 string, e.g., "A===".
+  const expectedEncodedLength = 4 * Math.ceil(decoded.length / 3);
+  if (encoded.length !== expectedEncodedLength) {
+    throw new Base64DecodeError('Invalid base64 string');
+  }
+
+  return decoded;
 }
 
 /** Converts a binary string to a Base64 encoded string. */

diff --git a/packages/firestore/src/platform/browser/base64.ts b/packages/firestore/src/platform/browser/base64.ts
@@ -15,9 +15,19 @@
  * limitations under the License.
  */
 
+import { Base64DecodeError } from '../../util/base64_decode_error';
+
 /** Converts a Base64 encoded string to a binary string. */
 export function decodeBase64(encoded: string): string {
-  return atob(encoded);
+  try {
+    return atob(encoded);
+  } catch (e) {
+    if (e instanceof DOMException) {
+      throw new Base64DecodeError('Invalid base64 string: ' + e);
+    } else {
+      throw e;
+    }
+  }
 }
 
 /** Converts a binary string to a Base64 encoded string. */

diff --git a/packages/firestore/src/platform/rn/base64.ts b/packages/firestore/src/platform/rn/base64.ts
@@ -15,21 +15,31 @@
  * limitations under the License.
  */
 
-import { base64 } from '@firebase/util';
+import { base64, DecodeBase64StringError } from '@firebase/util';
+
+import { Base64DecodeError } from '../../util/base64_decode_error';
 
 // WebSafe uses a different URL-encoding safe alphabet that doesn't match
 // the encoding used on the backend.
 const WEB_SAFE = false;
 
 /** Converts a Base64 encoded string to a binary string. */
 export function decodeBase64(encoded: string): string {
-  return String.fromCharCode.apply(
-    null,
-    // We use `decodeStringToByteArray()` instead of `decodeString()` since
-    // `decodeString()` returns Unicode strings, which doesn't match the values
-    // returned by `atob()`'s Latin1 representation.
-    base64.decodeStringToByteArray(encoded, WEB_SAFE)
-  );
+  try {
+    return String.fromCharCode.apply(
+      null,
+      // We use `decodeStringToByteArray()` instead of `decodeString()` since
+      // `decodeString()` returns Unicode strings, which doesn't match the values
+      // returned by `atob()`'s Latin1 representation.
+      base64.decodeStringToByteArray(encoded, WEB_SAFE)
+    );
+  } catch (e) {
+    if (e instanceof DecodeBase64StringError) {
+      throw new Base64DecodeError('Invalid base64 string: ' + e);
+    } else {
+      throw e;
+    }
+  }
 }
 
 /** Converts a binary string to a Base64 encoded string. */

diff --git a/packages/firestore/src/util/base64_decode_error.ts b/packages/firestore/src/util/base64_decode_error.ts
@@ -0,0 +1,23 @@
+/**
+ * @license
+ * Copyright 2023 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * An error encountered while decoding base64 string.
+ */
+export class Base64DecodeError extends Error {
+  readonly name = 'Base64DecodeError';
+}
diff --git a/packages/firestore/test/lite/integration.test.ts b/packages/firestore/test/lite/integration.test.ts
@@ -857,7 +857,7 @@ describe('DocumentSnapshot', () => {
 
   it('returns Bytes', () => {
     return withTestDocAndInitialData(
-      { bytes: Bytes.fromBase64String('aa') },
+      { bytes: Bytes.fromBase64String('aa==') },
       async docRef => {
         const docSnap = await getDoc(docRef);
         const bytes = docSnap.get('bytes');

diff --git a/packages/util/src/crypt.ts b/packages/util/src/crypt.ts
@@ -284,7 +284,7 @@ export const base64: Base64 = {
       ++i;
 
       if (byte1 == null || byte2 == null || byte3 == null || byte4 == null) {
-        throw Error();
+        throw new DecodeBase64StringError();
       }
 
       const outByte1 = (byte1 << 2) | (byte2 >> 4);
@@ -333,6 +333,13 @@ export const base64: Base64 = {
   }
 };
 
+/**
+ * An error encountered while decoding base64 string.
+ */
+export class DecodeBase64StringError extends Error {
+  readonly name = 'DecodeBase64StringError';
+}
+
 /**
  * URL-safe base64 encoding
  */