assafvayner HF Staff commited on
Commit
1c7b998
·
1 Parent(s): 8b1607e

fix(auth): bound scrypt params, cap concurrency, evict rate-limit keys, re-bootstrap admin

Browse files
src/lib/server/auth/password.ts CHANGED
@@ -5,10 +5,38 @@ const R = 8;
5
  const P = 1;
6
  const KEYLEN = 32;
7
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
8
  function scryptAsync(password: string, salt: Buffer, n: number, r: number, p: number): Promise<Buffer> {
9
- return new Promise((resolve, reject) => {
10
- scrypt(password, salt, KEYLEN, { N: n, r, p, maxmem: 128 * 1024 * 1024 }, (err, key) => (err ? reject(err) : resolve(key)));
11
- });
 
 
 
12
  }
13
 
14
  export const MIN_PASSWORD_LENGTH = 8;
@@ -26,6 +54,7 @@ export async function verifyPassword(password: string, stored: string): Promise<
26
  const r = Number(parts[2]);
27
  const p = Number(parts[3]);
28
  if (![n, r, p].every((x) => Number.isInteger(x) && x > 0)) return false;
 
29
  try {
30
  const salt = Buffer.from(parts[4], 'base64');
31
  const expected = Buffer.from(parts[5], 'base64');
 
5
  const P = 1;
6
  const KEYLEN = 32;
7
 
8
+ // Upper bounds on scrypt params accepted from a stored hash. A crafted users.json must not
9
+ // be able to turn verification into an unbounded CPU sink.
10
+ const MAX_N = 65536;
11
+ const MAX_R = 16;
12
+ const MAX_P = 4;
13
+
14
+ // Cap concurrent scrypt calls so a burst of logins can't exhaust CPU/memory; extra callers queue.
15
+ const MAX_CONCURRENT_SCRYPT = 4;
16
+ let activeScryptCalls = 0;
17
+ const scryptWaitQueue: Array<() => void> = [];
18
+
19
+ async function withScryptSlot<T>(fn: () => Promise<T>): Promise<T> {
20
+ if (activeScryptCalls >= MAX_CONCURRENT_SCRYPT) {
21
+ await new Promise<void>((resolve) => scryptWaitQueue.push(resolve));
22
+ }
23
+ activeScryptCalls++;
24
+ try {
25
+ return await fn();
26
+ } finally {
27
+ activeScryptCalls--;
28
+ const next = scryptWaitQueue.shift();
29
+ if (next) next();
30
+ }
31
+ }
32
+
33
  function scryptAsync(password: string, salt: Buffer, n: number, r: number, p: number): Promise<Buffer> {
34
+ return withScryptSlot(
35
+ () =>
36
+ new Promise((resolve, reject) => {
37
+ scrypt(password, salt, KEYLEN, { N: n, r, p, maxmem: 128 * 1024 * 1024 }, (err, key) => (err ? reject(err) : resolve(key)));
38
+ })
39
+ );
40
  }
41
 
42
  export const MIN_PASSWORD_LENGTH = 8;
 
54
  const r = Number(parts[2]);
55
  const p = Number(parts[3]);
56
  if (![n, r, p].every((x) => Number.isInteger(x) && x > 0)) return false;
57
+ if (n > MAX_N || r > MAX_R || p > MAX_P) return false;
58
  try {
59
  const salt = Buffer.from(parts[4], 'base64');
60
  const expected = Buffer.from(parts[5], 'base64');
src/lib/server/auth/rate-limit.ts CHANGED
@@ -1,3 +1,6 @@
 
 
 
1
  /** Sliding-window limiter: at most `max` allowed calls per key within `windowMs`. In-memory only. */
2
  export class RateLimiter {
3
  private hits = new Map<string, number[]>();
@@ -8,13 +11,25 @@ export class RateLimiter {
8
  private readonly now: () => number = Date.now
9
  ) {}
10
 
 
 
 
 
 
11
  allow(key: string): boolean {
12
  const t = this.now();
13
  const recent = (this.hits.get(key) ?? []).filter((h) => t - h < this.windowMs);
14
  if (recent.length >= this.max) {
15
- this.hits.set(key, recent);
 
16
  return false;
17
  }
 
 
 
 
 
 
18
  recent.push(t);
19
  this.hits.set(key, recent);
20
  return true;
 
1
+ /** Above this many tracked keys, prune stale entries on the next `allow` before inserting. */
2
+ const MAX_TRACKED_KEYS = 10_000;
3
+
4
  /** Sliding-window limiter: at most `max` allowed calls per key within `windowMs`. In-memory only. */
5
  export class RateLimiter {
6
  private hits = new Map<string, number[]>();
 
11
  private readonly now: () => number = Date.now
12
  ) {}
13
 
14
+ /** Number of keys currently tracked. Exposed for tests. */
15
+ get size(): number {
16
+ return this.hits.size;
17
+ }
18
+
19
  allow(key: string): boolean {
20
  const t = this.now();
21
  const recent = (this.hits.get(key) ?? []).filter((h) => t - h < this.windowMs);
22
  if (recent.length >= this.max) {
23
+ if (recent.length === 0) this.hits.delete(key);
24
+ else this.hits.set(key, recent);
25
  return false;
26
  }
27
+ if (this.hits.size > MAX_TRACKED_KEYS) {
28
+ for (const [k, hs] of this.hits) {
29
+ const mostRecent = hs.length > 0 ? Math.max(...hs) : -Infinity;
30
+ if (t - mostRecent >= this.windowMs) this.hits.delete(k);
31
+ }
32
+ }
33
  recent.push(t);
34
  this.hits.set(key, recent);
35
  return true;
src/lib/server/auth/session.ts CHANGED
@@ -20,6 +20,11 @@ export function createSessionToken(payload: Omit<SessionPayload, 'exp'>, secret:
20
  return `${data}.${sign(data, secret)}`;
21
  }
22
 
 
 
 
 
 
23
  export function verifySessionToken(token: string, secret: string, now = new Date()): SessionPayload | null {
24
  const dot = token.lastIndexOf('.');
25
  if (dot <= 0) return null;
 
20
  return `${data}.${sign(data, secret)}`;
21
  }
22
 
23
+ /**
24
+ * Verifies only the token's signature and expiry. This does NOT prove the session is still
25
+ * valid: callers MUST also check that `payload.uid` refers to a user that still exists, is
26
+ * active, and that `user.sessionVersion === payload.sv` (the hooks do this).
27
+ */
28
  export function verifySessionToken(token: string, secret: string, now = new Date()): SessionPayload | null {
29
  const dot = token.lastIndexOf('.');
30
  if (dot <= 0) return null;
src/lib/server/users/repository.ts CHANGED
@@ -43,10 +43,11 @@ export class UsersRepo {
43
  const clock = opts.clock ?? (() => new Date());
44
  const newId = opts.newId ?? ulid;
45
  const bytes = await store.get(USERS_PATH);
46
- if (bytes) {
47
- const file = UsersFileSchema.parse(JSON.parse(decodeText(bytes)));
48
- return new UsersRepo(store, file.users, clock, newId);
49
  }
 
50
  const now = clock().toISOString();
51
  const admin: User = {
52
  id: 'u_admin',
@@ -60,8 +61,8 @@ export class UsersRepo {
60
  createdAt: now,
61
  updatedAt: now
62
  };
63
- const repo = new UsersRepo(store, [], clock, newId);
64
- await repo.persist([admin]);
65
  return repo;
66
  }
67
 
@@ -83,7 +84,9 @@ export class UsersRepo {
83
  }
84
  /** All non-admin users, sorted by display name. */
85
  members(): User[] {
86
- return this.users.filter((u) => u.role === 'member').sort((a, b) => a.displayName.localeCompare(b.displayName));
 
 
87
  }
88
  isActiveMember(id: string): boolean {
89
  const u = this.byId(id);
 
43
  const clock = opts.clock ?? (() => new Date());
44
  const newId = opts.newId ?? ulid;
45
  const bytes = await store.get(USERS_PATH);
46
+ const existing = bytes ? UsersFileSchema.parse(JSON.parse(decodeText(bytes))).users : [];
47
+ if (bytes && existing.some((u) => u.role === 'admin')) {
48
+ return new UsersRepo(store, existing, clock, newId);
49
  }
50
+ // users.json is missing, or present but has no admin: bootstrap/re-bootstrap one.
51
  const now = clock().toISOString();
52
  const admin: User = {
53
  id: 'u_admin',
 
61
  createdAt: now,
62
  updatedAt: now
63
  };
64
+ const repo = new UsersRepo(store, existing, clock, newId);
65
+ await repo.persist([...existing, admin]);
66
  return repo;
67
  }
68
 
 
84
  }
85
  /** All non-admin users, sorted by display name. */
86
  members(): User[] {
87
+ return this.users
88
+ .filter((u) => u.role === 'member')
89
+ .sort((a, b) => a.displayName.localeCompare(b.displayName) || a.id.localeCompare(b.id));
90
  }
91
  isActiveMember(id: string): boolean {
92
  const u = this.byId(id);
tests/unit/auth/auth.test.ts CHANGED
@@ -18,6 +18,20 @@ describe('password', () => {
18
  it('rejects malformed hashes without throwing', async () => {
19
  expect(await verifyPassword('x', 'garbage')).toBe(false);
20
  });
 
 
 
 
 
 
 
 
 
 
 
 
 
 
21
  });
22
 
23
  describe('session token', () => {
@@ -51,4 +65,14 @@ describe('RateLimiter', () => {
51
  now = 60_001;
52
  expect(rl.allow('bob')).toBe(true);
53
  });
 
 
 
 
 
 
 
 
 
 
54
  });
 
18
  it('rejects malformed hashes without throwing', async () => {
19
  expect(await verifyPassword('x', 'garbage')).toBe(false);
20
  });
21
+
22
+ it('rejects stored hashes with out-of-bound scrypt params', async () => {
23
+ const placeholder = ['AAAAAAAAAAAAAAAAAAAAAA==', 'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA='];
24
+ const bigP = ['scrypt', 32768, 8, 64, ...placeholder].join('$');
25
+ expect(await verifyPassword('whatever', bigP)).toBe(false);
26
+ const bigN = ['scrypt', 131072, 8, 1, ...placeholder].join('$');
27
+ expect(await verifyPassword('whatever', bigN)).toBe(false);
28
+ });
29
+
30
+ it('resolves 10 concurrent hashPassword calls (semaphore sanity)', async () => {
31
+ const results = await Promise.all(Array.from({ length: 10 }, (_, i) => hashPassword(`pw-${i}`)));
32
+ expect(results).toHaveLength(10);
33
+ for (const hash of results) expect(hash.startsWith('scrypt$')).toBe(true);
34
+ });
35
  });
36
 
37
  describe('session token', () => {
 
65
  now = 60_001;
66
  expect(rl.allow('bob')).toBe(true);
67
  });
68
+
69
+ it('evicts stale keys once the tracked-key cap is exceeded', () => {
70
+ let now = 0;
71
+ const rl = new RateLimiter(5, 1_000, () => now);
72
+ for (let i = 0; i < 10_001; i++) rl.allow(`key-${i}`);
73
+ expect(rl.size).toBe(10_001);
74
+ now = 2_000; // window has fully passed for every key inserted above
75
+ expect(rl.allow('new-key')).toBe(true);
76
+ expect(rl.size).toBe(1);
77
+ });
78
  });
tests/unit/users/repository.test.ts CHANGED
@@ -4,6 +4,7 @@ import { join } from 'node:path';
4
  import { afterEach, beforeEach, describe, expect, it } from 'vitest';
5
  import { verifyPassword } from '../../../src/lib/server/auth/password';
6
  import { LocalFsStore } from '../../../src/lib/server/storage/local-fs-store';
 
7
  import { UsersRepo, UserValidationError } from '../../../src/lib/server/users/repository';
8
 
9
  describe('UsersRepo', () => {
@@ -79,6 +80,37 @@ describe('UsersRepo', () => {
79
  await expect(repo.rename(bob.id, '')).rejects.toBeInstanceOf(UserValidationError);
80
  });
81
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
82
  it('leaves memory untouched when the store write fails', async () => {
83
  const repo = await UsersRepo.load(store, { clock });
84
  store.put = async () => {
 
4
  import { afterEach, beforeEach, describe, expect, it } from 'vitest';
5
  import { verifyPassword } from '../../../src/lib/server/auth/password';
6
  import { LocalFsStore } from '../../../src/lib/server/storage/local-fs-store';
7
+ import { decodeText, encodeText } from '../../../src/lib/server/storage/store';
8
  import { UsersRepo, UserValidationError } from '../../../src/lib/server/users/repository';
9
 
10
  describe('UsersRepo', () => {
 
80
  await expect(repo.rename(bob.id, '')).rejects.toBeInstanceOf(UserValidationError);
81
  });
82
 
83
+ it('re-bootstraps a missing admin into an existing users.json, but leaves one with an admin untouched', async () => {
84
+ const member = {
85
+ id: 'u_bob',
86
+ username: 'bob',
87
+ displayName: 'Bob',
88
+ passwordHash: 'not-a-real-hash',
89
+ role: 'member' as const,
90
+ active: true,
91
+ mustChangePassword: false,
92
+ sessionVersion: 1,
93
+ createdAt: '2026-01-01T00:00:00.000Z',
94
+ updatedAt: '2026-01-01T00:00:00.000Z'
95
+ };
96
+ await store.put('users.json', encodeText(JSON.stringify({ version: 1, users: [member] })), 'application/json');
97
+
98
+ const repo = await UsersRepo.load(store, { clock });
99
+ expect(repo.all()).toHaveLength(2);
100
+ expect(repo.byUsername('admin')?.role).toBe('admin');
101
+ expect(repo.byUsername('bob')).toBeDefined();
102
+ const persisted = JSON.parse(decodeText((await store.get('users.json'))!));
103
+ expect(persisted.users).toHaveLength(2);
104
+
105
+ // a file that already has an admin is left untouched (no extra admin, no rewrite of members).
106
+ const withAdmin = repo.all();
107
+ const untouchedStore = new LocalFsStore(await mkdtemp(join(tmpdir(), 'splitwise-users-')));
108
+ await untouchedStore.put('users.json', encodeText(JSON.stringify({ version: 1, users: withAdmin })), 'application/json');
109
+ const reloaded = await UsersRepo.load(untouchedStore, { clock });
110
+ expect(reloaded.all()).toHaveLength(2);
111
+ expect(reloaded.all().filter((u) => u.role === 'admin')).toHaveLength(1);
112
+ });
113
+
114
  it('leaves memory untouched when the store write fails', async () => {
115
  const repo = await UsersRepo.load(store, { clock });
116
  store.put = async () => {