Skip to content

Commit 2511c02

Browse files
authored
fix: replace deprecated scmp with crypto.timingSafeEqual (#15322)
# Overview Removes the deprecated `scmp` package and replaces it with Node.js's native `crypto.timingSafeEqual()` for timing-safe password hash comparison. Resolves ECMS-356 ## Key Changes - **Replace scmp with crypto.timingSafeEqual** - Added length pre-check before comparison (required since `timingSafeEqual` throws on length mismatch, whereas `scmp` returned false) - **Remove scmp dependency** - Eliminates deprecation warning: "scmp@2.1.0: Just use Node.js's crypto.timingSafeEqual()" ## Design Decisions The length pre-check (`hashBuffer.length === storedHashBuffer.length`) preserves the original `scmp` behavior of returning false for mismatched lengths, rather than throwing an error. This maintains backward compatibility and security semantics. ## Overall Flow ```mermaid sequenceDiagram participant Client participant authenticateLocalStrategy participant crypto Client->>authenticateLocalStrategy: password + doc (hash, salt) authenticateLocalStrategy->>crypto: pbkdf2(password, salt) crypto-->>authenticateLocalStrategy: hashBuffer authenticateLocalStrategy->>authenticateLocalStrategy: Check buffer lengths match alt Lengths match authenticateLocalStrategy->>crypto: timingSafeEqual(hashBuffer, storedHashBuffer) crypto-->>authenticateLocalStrategy: boolean else Lengths differ authenticateLocalStrategy-->>Client: null (fail fast) end authenticateLocalStrategy-->>Client: doc | null ```
1 parent 3e27155 commit 2511c02

File tree

4 files changed

+75
-18
lines changed

4 files changed

+75
-18
lines changed

packages/payload/package.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,6 @@
125125
"qs-esm": "7.0.2",
126126
"range-parser": "1.2.1",
127127
"sanitize-filename": "1.6.3",
128-
"scmp": "2.1.0",
129128
"ts-essentials": "10.0.3",
130129
"tsx": "4.20.3",
131130
"undici": "7.18.2",
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import crypto from 'crypto'
2+
3+
import { describe, expect, it } from 'vitest'
4+
5+
import { authenticateLocalStrategy } from './authenticate.js'
6+
7+
// Helper to generate hash/salt like Payload does
8+
const generateHashAndSalt = (password: string): { hash: string; salt: string } => {
9+
const salt = crypto.randomBytes(32).toString('hex')
10+
const hash = crypto.pbkdf2Sync(password, salt, 25000, 512, 'sha256').toString('hex')
11+
return { hash, salt }
12+
}
13+
14+
describe('authenticateLocalStrategy', () => {
15+
it('should return doc when password is valid', async () => {
16+
const password = 'test-password'
17+
const { hash, salt } = generateHashAndSalt(password)
18+
const doc = { id: 1, hash, salt }
19+
20+
const result = await authenticateLocalStrategy({ doc, password })
21+
22+
expect(result).toEqual(doc)
23+
})
24+
25+
it('should return null when password is invalid', async () => {
26+
const { hash, salt } = generateHashAndSalt('correct-password')
27+
const doc = { id: 1, hash, salt }
28+
29+
const result = await authenticateLocalStrategy({ doc, password: 'wrong-password' })
30+
31+
expect(result).toBeNull()
32+
})
33+
34+
it('should return null when salt is missing', async () => {
35+
const { hash } = generateHashAndSalt('test-password')
36+
const doc = { id: 1, hash }
37+
38+
const result = await authenticateLocalStrategy({ doc, password: 'test-password' })
39+
40+
expect(result).toBeNull()
41+
})
42+
43+
it('should return null when hash is missing', async () => {
44+
const { salt } = generateHashAndSalt('test-password')
45+
const doc = { id: 1, salt }
46+
47+
const result = await authenticateLocalStrategy({ doc, password: 'test-password' })
48+
49+
expect(result).toBeNull()
50+
})
51+
52+
it('should return null when hash has different length (tampered)', async () => {
53+
const password = 'test-password'
54+
const { salt } = generateHashAndSalt(password)
55+
// Truncated hash - different length than expected 512 bytes
56+
const shortHash = 'abcd1234'
57+
const doc = { id: 1, hash: shortHash, salt }
58+
59+
const result = await authenticateLocalStrategy({ doc, password })
60+
61+
expect(result).toBeNull()
62+
})
63+
})

packages/payload/src/auth/strategies/local/authenticate.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
// @ts-strict-ignore
22
import crypto from 'crypto'
3-
// @ts-expect-error - no types available
4-
import scmp from 'scmp'
53

64
import type { TypeWithID } from '../../../collections/config/types.js'
75

@@ -23,7 +21,12 @@ export const authenticateLocalStrategy = async ({ doc, password }: Args): Promis
2321
reject(e)
2422
}
2523

26-
if (scmp(hashBuffer, Buffer.from(hash, 'hex'))) {
24+
const storedHashBuffer = Buffer.from(hash, 'hex')
25+
26+
if (
27+
hashBuffer.length === storedHashBuffer.length &&
28+
crypto.timingSafeEqual(hashBuffer, storedHashBuffer)
29+
) {
2730
resolve(doc)
2831
} else {
2932
reject(new Error('Invalid password'))

pnpm-lock.yaml

Lines changed: 6 additions & 14 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)