Skip to content

Commit 386a989

Browse files
authored
fix: Validate token type in PagesRouter to prevent type confusion errors (#10212)
1 parent 9c48765 commit 386a989

File tree

3 files changed

+59
-10
lines changed

3 files changed

+59
-10
lines changed

spec/PagesRouter.spec.js

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -990,6 +990,56 @@ describe('Pages Router', () => {
990990
await fs.rm(baseDir, { recursive: true, force: true });
991991
}
992992
});
993+
994+
it('rejects non-string token in verifyEmail', async () => {
995+
await reconfigureServer(config);
996+
const url = `${config.publicServerURL}/apps/test/verify_email?token[toString]=abc`;
997+
const response = await request({
998+
url: url,
999+
followRedirects: false,
1000+
}).catch(e => e);
1001+
expect(response.status).not.toBe(500);
1002+
});
1003+
1004+
it('rejects non-string token in requestResetPassword', async () => {
1005+
await reconfigureServer(config);
1006+
const url = `${config.publicServerURL}/apps/test/request_password_reset?token[toString]=abc`;
1007+
const response = await request({
1008+
url: url,
1009+
followRedirects: false,
1010+
}).catch(e => e);
1011+
expect(response.status).not.toBe(500);
1012+
});
1013+
1014+
it('rejects non-string token in resetPassword via POST', async () => {
1015+
await reconfigureServer(config);
1016+
const url = `${config.publicServerURL}/apps/test/request_password_reset`;
1017+
const response = await request({
1018+
method: 'POST',
1019+
url: url,
1020+
headers: {
1021+
'Content-Type': 'application/json',
1022+
},
1023+
body: { token: { toString: 'abc' }, new_password: 'newpass123' },
1024+
followRedirects: false,
1025+
}).catch(e => e);
1026+
expect(response.status).not.toBe(500);
1027+
});
1028+
1029+
it('rejects non-string token in resendVerificationEmail via POST', async () => {
1030+
await reconfigureServer(config);
1031+
const url = `${config.publicServerURL}/apps/test/resend_verification_email`;
1032+
const response = await request({
1033+
method: 'POST',
1034+
url: url,
1035+
headers: {
1036+
'Content-Type': 'application/json',
1037+
},
1038+
body: { token: { toString: 'abc' } },
1039+
followRedirects: false,
1040+
}).catch(e => e);
1041+
expect(response.status).not.toBe(500);
1042+
});
9931043
});
9941044

9951045
describe('custom route', () => {

spec/RegexVulnerabilities.spec.js

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -345,10 +345,10 @@ describe('Regex Vulnerabilities', () => {
345345
describe('on resend verification email', () => {
346346
// The PagesRouter uses express.urlencoded({ extended: false }) which does not parse
347347
// nested objects (e.g. token[$regex]=^.), so the HTTP layer already blocks object injection.
348-
// The toString() guard in resendVerificationEmail() is defense-in-depth in case the
349-
// body parser configuration changes. These tests verify the guard works correctly
348+
// Non-string tokens are rejected (treated as undefined) to prevent both NoSQL injection
349+
// and type confusion errors. These tests verify the guard works correctly
350350
// by directly testing the PagesRouter method.
351-
it('should sanitize non-string token to string via toString()', async () => {
351+
it('should reject non-string token as undefined', async () => {
352352
const { PagesRouter } = require('../lib/Routers/PagesRouter');
353353
const router = new PagesRouter();
354354
const goToPage = spyOn(router, 'goToPage').and.returnValue(Promise.resolve());
@@ -363,10 +363,9 @@ describe('Regex Vulnerabilities', () => {
363363
},
364364
};
365365
await router.resendVerificationEmail(req);
366-
// The token passed to userController.resendVerificationEmail should be a string
366+
// Non-string token should be treated as undefined
367367
const passedToken = resendSpy.calls.first().args[2];
368-
expect(typeof passedToken).toEqual('string');
369-
expect(passedToken).toEqual('[object Object]');
368+
expect(passedToken).toBeUndefined();
370369
});
371370

372371
it('should pass through valid string token unchanged', async () => {

src/Routers/PagesRouter.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ export class PagesRouter extends PromiseRouter {
8484
verifyEmail(req) {
8585
const config = req.config;
8686
const { token: rawToken } = req.query;
87-
const token = rawToken && typeof rawToken !== 'string' ? rawToken.toString() : rawToken;
87+
const token = typeof rawToken === 'string' ? rawToken : undefined;
8888

8989
if (!config) {
9090
this.invalidRequest();
@@ -109,7 +109,7 @@ export class PagesRouter extends PromiseRouter {
109109
const config = req.config;
110110
const username = req.body?.username;
111111
const rawToken = req.body?.token;
112-
const token = rawToken && typeof rawToken !== 'string' ? rawToken.toString() : rawToken;
112+
const token = typeof rawToken === 'string' ? rawToken : undefined;
113113

114114
if (!config) {
115115
this.invalidRequest();
@@ -151,7 +151,7 @@ export class PagesRouter extends PromiseRouter {
151151
}
152152

153153
const { token: rawToken } = req.query;
154-
const token = rawToken && typeof rawToken !== 'string' ? rawToken.toString() : rawToken;
154+
const token = typeof rawToken === 'string' ? rawToken : undefined;
155155

156156
if (!token) {
157157
return this.goToPage(req, pages.passwordResetLinkInvalid);
@@ -180,7 +180,7 @@ export class PagesRouter extends PromiseRouter {
180180
}
181181

182182
const { new_password, token: rawToken } = req.body || {};
183-
const token = rawToken && typeof rawToken !== 'string' ? rawToken.toString() : rawToken;
183+
const token = typeof rawToken === 'string' ? rawToken : undefined;
184184

185185
if ((!token || !new_password) && req.xhr === false) {
186186
return this.goToPage(req, pages.passwordResetLinkInvalid);

0 commit comments

Comments
 (0)