Skip to content

Commit 2e8f471

Browse files
committed
fix: prevent CORS policy header leakage for rejected origins
Move CORS policy headers (Allow-Methods, Allow-Headers, Max-Age, Allow-Credentials) inside the origin-allowed check so they are not sent to disallowed origins during preflight responses.
1 parent 5ce4d25 commit 2e8f471

2 files changed

Lines changed: 50 additions & 14 deletions

File tree

lib/middleware/cors.js

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -131,24 +131,24 @@ function createCORS(options = {}) {
131131
if (typeof origin === 'function' || Array.isArray(origin)) {
132132
response.headers.set('Vary', 'Origin')
133133
}
134-
}
135134

136-
// Don't allow wildcard origin with credentials
137-
if (credentials && allowedOrigin !== '*') {
138-
response.headers.set('Access-Control-Allow-Credentials', 'true')
139-
}
135+
// Don't allow wildcard origin with credentials
136+
if (credentials && allowedOrigin !== '*') {
137+
response.headers.set('Access-Control-Allow-Credentials', 'true')
138+
}
140139

141-
response.headers.set(
142-
'Access-Control-Allow-Methods',
143-
(Array.isArray(methods) ? methods : [methods]).join(', '),
144-
)
140+
response.headers.set(
141+
'Access-Control-Allow-Methods',
142+
(Array.isArray(methods) ? methods : [methods]).join(', '),
143+
)
145144

146-
response.headers.set(
147-
'Access-Control-Allow-Headers',
148-
allowedHeadersList.join(', '),
149-
)
145+
response.headers.set(
146+
'Access-Control-Allow-Headers',
147+
allowedHeadersList.join(', '),
148+
)
150149

151-
response.headers.set('Access-Control-Max-Age', maxAge.toString())
150+
response.headers.set('Access-Control-Max-Age', maxAge.toString())
151+
}
152152

153153
if (preflightContinue) {
154154
const result = next()

test/unit/cors.test.js

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -683,12 +683,48 @@ describe('CORS Middleware', () => {
683683

684684
const middleware = cors({
685685
origin: false, // Explicitly set to false
686+
credentials: true,
687+
})
688+
689+
const response = await middleware(req, next)
690+
691+
expect(response.status).toBe(204)
692+
expect(response.headers.get('Access-Control-Allow-Origin')).toBeNull()
693+
// Should not leak CORS policy headers to disallowed origins
694+
expect(response.headers.get('Access-Control-Allow-Methods')).toBeNull()
695+
expect(response.headers.get('Access-Control-Allow-Headers')).toBeNull()
696+
expect(response.headers.get('Access-Control-Max-Age')).toBeNull()
697+
expect(
698+
response.headers.get('Access-Control-Allow-Credentials'),
699+
).toBeNull()
700+
expect(next).not.toHaveBeenCalled()
701+
})
702+
703+
it('should not leak CORS headers in preflight for rejected origins', async () => {
704+
req = createTestRequest('OPTIONS', '/api/test')
705+
req.headers = new Headers({
706+
Origin: 'https://evil.com',
707+
'Access-Control-Request-Method': 'POST',
708+
})
709+
710+
const middleware = cors({
711+
origin: ['https://trusted.com'],
712+
credentials: true,
713+
maxAge: 3600,
714+
methods: ['GET', 'POST'],
715+
allowedHeaders: ['Content-Type', 'Authorization'],
686716
})
687717

688718
const response = await middleware(req, next)
689719

690720
expect(response.status).toBe(204)
691721
expect(response.headers.get('Access-Control-Allow-Origin')).toBeNull()
722+
expect(response.headers.get('Access-Control-Allow-Methods')).toBeNull()
723+
expect(response.headers.get('Access-Control-Allow-Headers')).toBeNull()
724+
expect(response.headers.get('Access-Control-Max-Age')).toBeNull()
725+
expect(
726+
response.headers.get('Access-Control-Allow-Credentials'),
727+
).toBeNull()
692728
expect(next).not.toHaveBeenCalled()
693729
})
694730

0 commit comments

Comments
 (0)