Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions spec/PagesRouter.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -990,6 +990,56 @@ describe('Pages Router', () => {
await fs.rm(baseDir, { recursive: true, force: true });
}
});

it('rejects non-string token in verifyEmail', async () => {
await reconfigureServer(config);
const url = `${config.publicServerURL}/apps/test/verify_email?token[toString]=abc`;
const response = await request({
url: url,
followRedirects: false,
}).catch(e => e);
expect(response.status).not.toBe(500);
});

it('rejects non-string token in requestResetPassword', async () => {
await reconfigureServer(config);
const url = `${config.publicServerURL}/apps/test/request_password_reset?token[toString]=abc`;
const response = await request({
url: url,
followRedirects: false,
}).catch(e => e);
expect(response.status).not.toBe(500);
});

it('rejects non-string token in resetPassword via POST', async () => {
await reconfigureServer(config);
const url = `${config.publicServerURL}/apps/test/request_password_reset`;
const response = await request({
method: 'POST',
url: url,
headers: {
'Content-Type': 'application/json',
},
body: { token: { toString: 'abc' }, new_password: 'newpass123' },
followRedirects: false,
}).catch(e => e);
expect(response.status).not.toBe(500);
});

it('rejects non-string token in resendVerificationEmail via POST', async () => {
await reconfigureServer(config);
const url = `${config.publicServerURL}/apps/test/resend_verification_email`;
const response = await request({
method: 'POST',
url: url,
headers: {
'Content-Type': 'application/json',
},
body: { token: { toString: 'abc' } },
followRedirects: false,
}).catch(e => e);
expect(response.status).not.toBe(500);
});
});

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

it('should pass through valid string token unchanged', async () => {
Expand Down
8 changes: 4 additions & 4 deletions src/Routers/PagesRouter.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export class PagesRouter extends PromiseRouter {
verifyEmail(req) {
const config = req.config;
const { token: rawToken } = req.query;
const token = rawToken && typeof rawToken !== 'string' ? rawToken.toString() : rawToken;
const token = typeof rawToken === 'string' ? rawToken : undefined;

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

if (!config) {
this.invalidRequest();
Expand Down Expand Up @@ -151,7 +151,7 @@ export class PagesRouter extends PromiseRouter {
}

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

if (!token) {
return this.goToPage(req, pages.passwordResetLinkInvalid);
Expand Down Expand Up @@ -180,7 +180,7 @@ export class PagesRouter extends PromiseRouter {
}

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

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