fix(security):harden against pathological inputs

packages/kdbx/src/bytes.ts

@@ -186,6 +186,16 @@ export class ByteReader {
   }
 
   #require(count: number): void {
+    // A negative count (e.g. from a maliciously-crafted Int32 length field in
+    // a TLV-framed structure — the outer/inner header and VariantDictionary
+    // parsers all read attacker-controlled lengths this way) would otherwise
+    // pass the check below, since offset + a negative count is always less
+    // than offset. readBytes would then walk #offset *backward* instead of
+    // throwing, which a crafted file can use to pin the cursor at a fixed
+    // position and loop forever.
+    if (count < 0) {
+      throw new RangeError(`byte count must not be negative, got ${count}`);
+    }
     if (this.#offset + count > this.#bytes.length) {
       throw new RangeError(
         `unexpected end of data: needed ${count} byte(s) at offset ${this.#offset}`,

packages/kdbx/src/crypto.ts

@@ -96,6 +96,19 @@ export async function aesCbcDecrypt(
   }
 }
 
+/**
+ * Sanity ceiling on AES-KDF's `rounds` parameter, which for KDBX 3.1 (and the
+ * AES-KDF choice in KDBX 4.x KDF parameters) comes straight from the file's
+ * own, not-yet-authenticated header. `transformHalf` below allocates a
+ * `16 * rounds`-byte plaintext buffer per half, so an unchecked value near
+ * Number.MAX_SAFE_INTEGER (the previous ceiling) would attempt a many-exabyte
+ * allocation just from opening a crafted file, before any credentials are
+ * checked. This value is far above any real KeePass configuration (this
+ * project's own default is 60,000) while keeping a worst-case unlock attempt
+ * bounded to something a browser tab can actually survive.
+ */
+const KX_MAX_AES_KDF_ROUNDS = 100_000_000n;
+
 /**
  * AES-KDF transformation (the KDBX 3.1 / legacy key derivation function).
  *
@@ -116,8 +129,10 @@ export async function aesKdfTransform(
   if (rounds <= 0n) {
     throw new RangeError('AES-KDF rounds must be positive');
   }
-  if (rounds > BigInt(Number.MAX_SAFE_INTEGER)) {
-    throw new RangeError('AES-KDF rounds exceed the supported maximum');
+  if (rounds > KX_MAX_AES_KDF_ROUNDS) {
+    throw new RangeError(
+      `AES-KDF rounds (${rounds}) exceed the maximum this app will run (${KX_MAX_AES_KDF_ROUNDS})`,
+    );
   }
   const n = Number(rounds);
   const subtle = kx_getCrypto().subtle;

packages/kdbx/src/kdf.ts

@@ -16,6 +16,23 @@ import { type VariantDictionary, vdRequireBytes, vdRequireInt } from './variant-
 const KX_ARGON2_TAG_LENGTH = 32;
 const KX_BYTES_PER_KIB = 1024n;
 
+/**
+ * Sanity ceiling on Argon2 cost parameters read from a file's (untrusted,
+ * pre-authentication) KDF parameters. RFC 9106's own legal ranges — enforced
+ * separately by the argon2 package itself — allow up to 2^32-1 KiB (~4 TiB)
+ * of memory and 2^32-1 iterations, since that package has no opinion on what
+ * a caller finds reasonable. Left unchecked here, a crafted file could force
+ * an attempted multi-gigabyte-or-larger allocation, or a computation that
+ * never realistically finishes, just by being opened — with any password
+ * attempt, since the KDF runs before the file's authenticity is verified.
+ * These values are generous relative to any real-world KDBX configuration
+ * (KeePass's own defaults are far below them) while keeping a worst-case
+ * unlock attempt bounded to something a browser tab can actually survive.
+ */
+const KX_MAX_ARGON2_MEMORY_KIB = 2 * 1024 * 1024; // 2 GiB
+const KX_MAX_ARGON2_ITERATIONS = 64;
+const KX_MAX_ARGON2_PARALLELISM = 64;
+
 /** Transform a 32-byte composite key with AES-KDF (KDBX 3.1 and the AES-KDF KDF). */
 export async function aesKdf(
   compositeKey: Uint8Array,
@@ -71,6 +88,22 @@ function kx_runArgon2(
   const memoryBytes = vdRequireInt(params, KdfParam.Argon2Memory);
   const memory = Number(memoryBytes / KX_BYTES_PER_KIB);
 
+  if (parallelism > KX_MAX_ARGON2_PARALLELISM) {
+    throw new Error(
+      `Argon2 parallelism (${parallelism}) exceeds the maximum this app will run (${KX_MAX_ARGON2_PARALLELISM})`,
+    );
+  }
+  if (iterations > KX_MAX_ARGON2_ITERATIONS) {
+    throw new Error(
+      `Argon2 iterations (${iterations}) exceeds the maximum this app will run (${KX_MAX_ARGON2_ITERATIONS})`,
+    );
+  }
+  if (memory > KX_MAX_ARGON2_MEMORY_KIB) {
+    throw new Error(
+      `Argon2 memory (${memory} KiB) exceeds the maximum this app will run (${KX_MAX_ARGON2_MEMORY_KIB} KiB)`,
+    );
+  }
+
   const versionParam = params.get(KdfParam.Argon2Version);
   const version =
     versionParam === undefined

packages/kdbx/tests/aes-kdf.test.ts

@@ -54,3 +54,15 @@ test('aesKdfTransform validates input', async () => {
     RangeError,
   );
 });
+
+test('aesKdfTransform rejects a rounds count above the sanity ceiling without attempting it', async () => {
+  // rounds comes straight from a file's own (not-yet-authenticated) header;
+  // this must reject immediately rather than actually attempting anywhere
+  // near this many AES rounds, or the test would never finish either.
+  const key = new Uint8Array(32);
+  const seed = new Uint8Array(32);
+  await assert.rejects(
+    () => aesKdfTransform(key, seed, 100_000_001n),
+    /rounds.*exceed the maximum/,
+  );
+});

packages/kdbx/tests/bytes.test.ts

@@ -72,6 +72,18 @@ test('ByteReader throws past the end', () => {
   assert.throws(() => new ByteReader(new Uint8Array(2)).readU32(), RangeError);
 });
 
+test('ByteReader rejects a negative byte count instead of seeking backward', () => {
+  // A crafted TLV length field (e.g. a header field's Int32 size) can be
+  // negative. Without a check, offset + count still passes the "not past the
+  // end" bound, and the offset would walk backward instead of erroring —
+  // letting a malicious file pin the cursor in place and loop forever.
+  const reader = new ByteReader(new Uint8Array(16));
+  reader.readBytes(8); // advance the cursor away from 0 first
+  assert.throws(() => reader.readBytes(-1), /byte count must not be negative/);
+  // The failed read must not have moved the cursor.
+  assert.equal(reader.offset, 8);
+});
+
 test('concatBytes and bytesEqual', () => {
   assert.deepEqual(
     concatBytes(new Uint8Array([1]), new Uint8Array([2, 3])),

packages/kdbx/tests/kdf.test.ts

@@ -68,3 +68,37 @@ test('Argon2 ignores a secret/associated-data parameter stored as the wrong type
   const key2 = await transformWithKdfParameters(new Uint8Array(32), plain);
   assert.deepEqual(key1, key2); // the malformed secret param was ignored
 });
+
+// A file's KDF parameters are read before its authenticity is checked (the
+// derived key is needed to verify the HMAC in the first place), so an
+// untrusted file can request any cost it likes. These three guard against
+// that being used to force a multi-gigabyte allocation or a computation that
+// never finishes just by being opened, regardless of the password typed.
+
+test('Argon2 rejects a parallelism above the sanity ceiling', async () => {
+  const params = argon2Params([[KdfParam.Argon2Parallelism, { type: 'uint32', value: 65 }]]);
+  await assert.rejects(
+    () => transformWithKdfParameters(new Uint8Array(32), params),
+    /parallelism.*exceeds the maximum/,
+  );
+});
+
+test('Argon2 rejects an iteration count above the sanity ceiling', async () => {
+  const params = argon2Params([[KdfParam.Argon2Iterations, { type: 'uint64', value: 65n }]]);
+  await assert.rejects(
+    () => transformWithKdfParameters(new Uint8Array(32), params),
+    /iterations.*exceeds the maximum/,
+  );
+});
+
+test('Argon2 rejects a memory cost above the sanity ceiling', async () => {
+  // KDBX stores memory in bytes; one KiB over the 2 GiB ceiling, rejected
+  // before any allocation is attempted.
+  const params = argon2Params([
+    [KdfParam.Argon2Memory, { type: 'uint64', value: 2n * 1024n * 1024n * 1024n + 1024n }],
+  ]);
+  await assert.rejects(
+    () => transformWithKdfParameters(new Uint8Array(32), params),
+    /memory.*exceeds the maximum/,
+  );
+});

pages/0x67/logic.ts

@@ -255,13 +255,26 @@ function exportFields(entry: XmlElement, group: XmlElement): [string, string][]
   ];
 }
 
+/** A leading `=`, `+`, `-`, `@`, tab, or CR makes Excel, Sheets, and
+ * LibreOffice Calc treat a CSV field as a formula rather than literal text —
+ * CWE-1236. Entry data (a Title, URL, or Notes field) is attacker-reachable
+ * in a way a spreadsheet's own cells normally aren't, so it can't be assumed
+ * safe. */
+const CSV_FORMULA_TRIGGER = /^[=+\-@\t\r]/;
+
 /** Quote a CSV field only when it needs it (contains a comma, quote, or
- * newline), doubling any internal quotes — RFC 4180. */
+ * newline), doubling any internal quotes — RFC 4180. RFC 4180 quoting alone
+ * does not stop a spreadsheet application from evaluating a quoted field's
+ * content as a formula, so a leading formula-trigger character is neutralized
+ * first by prefixing a literal apostrophe — the standard mitigation every
+ * mainstream spreadsheet app already treats as "force text" (the same effect
+ * as typing `'123` into a cell by hand). */
 function csvField(value: string): string {
-  if (/[",\r\n]/.test(value)) {
-    return `"${value.replace(/"/g, '""')}"`;
+  const safe = CSV_FORMULA_TRIGGER.test(value) ? `'${value}` : value;
+  if (/[",\r\n]/.test(safe)) {
+    return `"${safe.replace(/"/g, '""')}"`;
   }
-  return value;
+  return safe;
 }
 
 /** Serialize entries (Group, Title, UserName, Password, URL, Notes, Tags) as

pages/tests/0x67-logic.test.ts

@@ -309,6 +309,45 @@ test('toCsv writes just the header row for an empty entry list', () => {
   assert.equal(toCsv([]), 'Group,Title,UserName,Password,URL,Notes,Tags');
 });
 
+test('toCsv neutralizes a leading formula-trigger character (CSV/Excel formula injection)', () => {
+  const root = createGroup('Personal');
+  const entry = createEntry({
+    title: '=1+1',
+    username: '+15555550100',
+    password: '-hunter2',
+    url: '@example.com',
+  });
+  appendChild(root, entry);
+
+  const csv = toCsv(collectAllEntries(root));
+  const lines = csv.split('\r\n');
+  assert.equal(lines[1], "Personal,'=1+1,'+15555550100,'-hunter2,'@example.com,,");
+});
+
+test('toCsv combines formula neutralization with RFC 4180 quoting when a field needs both', () => {
+  const root = createGroup('Personal');
+  const entry = createEntry({ title: '=SUM(A1,"x")' });
+  appendChild(root, entry);
+
+  const csv = toCsv(collectAllEntries(root));
+  const lines = csv.split('\r\n');
+  // The apostrophe is added first; the (now longer) value still contains a
+  // comma and a quote, so it's also RFC 4180 quoted, same as any other field.
+  assert.equal(lines[1], `Personal,"'=SUM(A1,""x"")",,,,,`);
+});
+
+test('toCsv leaves a field with no leading formula-trigger character untouched', () => {
+  const root = createGroup('Personal');
+  // '@' appears, but not leading — only a *leading* trigger character is
+  // dangerous to a spreadsheet, so this must not be prefixed.
+  const entry = createEntry({ username: 'plain@example.com' });
+  appendChild(root, entry);
+
+  const csv = toCsv(collectAllEntries(root));
+  const lines = csv.split('\r\n');
+  assert.equal(lines[1], 'Personal,(no title),plain@example.com,,,,');
+});
+
 test('toXml escapes XML-significant characters and wraps entries in <Entries>', () => {
   const root = createGroup('Root');
   const entry = createEntry({ title: 'A & B <script> "quote" \'apos\'' });