From ccd90343394f433b287733ad0a33c08e0a71f53c Mon Sep 17 00:00:00 2001 From: Lauri Ojansivu Date: Sun, 2 Nov 2025 11:42:07 +0200 Subject: [PATCH] Fix SECURITY ISSUE 5: Attachment API uses bearer value as userId and DoS (Low). Thanks to Siam Thanat Hack (STH) and xet7 ! --- SECURITY.md | 15 ++ server/lib/tests/attachmentApi.tests.js | 203 ++++++++++++++++++++++++ server/lib/tests/index.js | 1 + server/routes/attachmentApi.js | 104 ++++++++++-- 4 files changed, 312 insertions(+), 11 deletions(-) create mode 100644 server/lib/tests/attachmentApi.tests.js diff --git a/SECURITY.md b/SECURITY.md index f93b34ac8..0ad7a0256 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -208,6 +208,21 @@ Meteor.startup(() => { - Only the caller's own userId is added/removed from the selected estimation bucket (e.g., one, two, five, etc.). - Methods cover setting/unsetting poker question/end, casting votes, replaying, and setting final estimation. +## Attachment API: authentication and DoS prevention + +- The attachment API (`/api/attachment/*`) requires proper authentication using `X-User-Id` and `X-Auth-Token` headers. +- Authentication validates tokens by hashing with `Accounts._hashLoginToken` and matching against stored login tokens, preventing identity spoofing. +- Request handlers implement: + - 30-second timeout to prevent hanging connections. + - Request body size limits (50MB for uploads, 10MB for metadata operations). + - Proper error handling and guaranteed response completion. + - Request error event handlers to clean up failed connections. +- This prevents: + - DoS attacks via concurrent unauthenticated or malformed requests. + - Identity spoofing by using arbitrary bearer tokens or user IDs. + - Resource exhaustion from hanging connections or excessive payloads. +- Access control: all attachment operations verify board membership before allowing access. + ## Brute force login protection - https://github.com/wekan/wekan/commit/23e5e1e3bd081699ce39ce5887db7e612616014d diff --git a/server/lib/tests/attachmentApi.tests.js b/server/lib/tests/attachmentApi.tests.js new file mode 100644 index 000000000..1b89c236a --- /dev/null +++ b/server/lib/tests/attachmentApi.tests.js @@ -0,0 +1,203 @@ +/* eslint-env mocha */ +import { expect } from 'chai'; +import sinon from 'sinon'; +import { Meteor } from 'meteor/meteor'; +import { Accounts } from 'meteor/accounts-base'; + +describe('attachmentApi authentication', function() { + let findOneStub, hashStub; + + beforeEach(function() { + hashStub = sinon.stub(Accounts, '_hashLoginToken'); + findOneStub = sinon.stub(Meteor.users, 'findOne'); + }); + + afterEach(function() { + if (hashStub) hashStub.restore(); + if (findOneStub) findOneStub.restore(); + }); + + // Mock request/response objects + function createMockReq(headers = {}) { + return { + headers, + on: sinon.stub(), + connection: { destroy: sinon.stub() }, + }; + } + + function createMockRes() { + return { + writeHead: sinon.stub(), + end: sinon.stub(), + headersSent: false, + }; + } + + describe('authenticateApiRequest', function() { + it('denies request with missing X-User-Id header', function() { + const req = createMockReq({ 'x-auth-token': 'sometoken' }); + const res = createMockRes(); + + // Simulate the handler behavior + let errorThrown = false; + try { + if (!req.headers['x-user-id'] || !req.headers['x-auth-token']) { + throw new Meteor.Error('unauthorized', 'Missing X-User-Id or X-Auth-Token headers'); + } + } catch (error) { + errorThrown = true; + expect(error.error).to.equal('unauthorized'); + } + + expect(errorThrown).to.equal(true); + }); + + it('denies request with missing X-Auth-Token header', function() { + const req = createMockReq({ 'x-user-id': 'user123' }); + + let errorThrown = false; + try { + if (!req.headers['x-user-id'] || !req.headers['x-auth-token']) { + throw new Meteor.Error('unauthorized', 'Missing X-User-Id or X-Auth-Token headers'); + } + } catch (error) { + errorThrown = true; + expect(error.error).to.equal('unauthorized'); + } + + expect(errorThrown).to.equal(true); + }); + + it('denies request with invalid token', function() { + const userId = 'user123'; + const token = 'invalidtoken'; + const req = createMockReq({ 'x-user-id': userId, 'x-auth-token': token }); + + hashStub.returns('hashedInvalidToken'); + findOneStub.returns(null); // No user found + + let errorThrown = false; + try { + const hashedToken = Accounts._hashLoginToken(token); + const user = Meteor.users.findOne({ + _id: userId, + 'services.resume.loginTokens.hashedToken': hashedToken, + }); + if (!user) { + throw new Meteor.Error('unauthorized', 'Invalid credentials'); + } + } catch (error) { + errorThrown = true; + expect(error.error).to.equal('unauthorized'); + } + + expect(errorThrown).to.equal(true); + expect(hashStub.calledOnce).to.equal(true); + expect(findOneStub.calledOnce).to.equal(true); + }); + + it('allows request with valid X-User-Id and X-Auth-Token', function() { + const userId = 'user123'; + const token = 'validtoken'; + const req = createMockReq({ 'x-user-id': userId, 'x-auth-token': token }); + + const hashedToken = 'hashedValidToken'; + hashStub.returns(hashedToken); + findOneStub.returns({ _id: userId }); // User found + + let authenticatedUserId = null; + try { + const hashed = Accounts._hashLoginToken(token); + const user = Meteor.users.findOne({ + _id: userId, + 'services.resume.loginTokens.hashedToken': hashed, + }); + if (!user) { + throw new Meteor.Error('unauthorized', 'Invalid credentials'); + } + authenticatedUserId = userId; + } catch (error) { + // Should not throw + } + + expect(authenticatedUserId).to.equal(userId); + expect(hashStub.calledOnce).to.equal(true); + expect(hashStub.calledWith(token)).to.equal(true); + expect(findOneStub.calledOnce).to.equal(true); + const queryArg = findOneStub.getCall(0).args[0]; + expect(queryArg._id).to.equal(userId); + expect(queryArg['services.resume.loginTokens.hashedToken']).to.equal(hashedToken); + }); + + it('prevents identity spoofing by validating hashed token', function() { + const victimId = 'victim-user-id'; + const attackerToken = 'attacker-token'; + const req = createMockReq({ 'x-user-id': victimId, 'x-auth-token': attackerToken }); + + hashStub.returns('hashedAttackerToken'); + // Simulate victim exists but token doesn't match + findOneStub.returns(null); + + let errorThrown = false; + try { + const hashed = Accounts._hashLoginToken(attackerToken); + const user = Meteor.users.findOne({ + _id: victimId, + 'services.resume.loginTokens.hashedToken': hashed, + }); + if (!user) { + throw new Meteor.Error('unauthorized', 'Invalid credentials'); + } + } catch (error) { + errorThrown = true; + expect(error.error).to.equal('unauthorized'); + } + + expect(errorThrown).to.equal(true); + }); + }); + + describe('request handler DoS prevention', function() { + it('enforces timeout on hanging requests', function(done) { + this.timeout(5000); + + const req = createMockReq({ 'x-user-id': 'user1', 'x-auth-token': 'token1' }); + const res = createMockRes(); + + // Simulate timeout behavior + const timeout = setTimeout(() => { + if (!res.headersSent) { + res.headersSent = true; + res.writeHead(408, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify({ success: false, error: 'Request timeout' })); + } + }, 100); // Short timeout for test + + // Wait for timeout + setTimeout(() => { + expect(res.headersSent).to.equal(true); + expect(res.writeHead.calledWith(408)).to.equal(true); + clearTimeout(timeout); + done(); + }, 150); + }); + + it('limits request body size', function() { + const req = createMockReq({ 'x-user-id': 'user1', 'x-auth-token': 'token1' }); + let body = ''; + const limit = 50 * 1024 * 1024; // 50MB + + // Simulate exceeding limit + body = 'a'.repeat(limit + 1); + expect(body.length).to.be.greaterThan(limit); + + // Handler should destroy connection + if (body.length > limit) { + req.connection.destroy(); + } + + expect(req.connection.destroy.calledOnce).to.equal(true); + }); + }); +}); diff --git a/server/lib/tests/index.js b/server/lib/tests/index.js index 7b8ef48ed..d3c2870eb 100644 --- a/server/lib/tests/index.js +++ b/server/lib/tests/index.js @@ -3,3 +3,4 @@ import './users.security.tests'; import './boards.security.tests'; import './cards.security.tests'; import './cards.methods.tests'; +import './attachmentApi.tests'; diff --git a/server/routes/attachmentApi.js b/server/routes/attachmentApi.js index 51f18082c..d08196d19 100644 --- a/server/routes/attachmentApi.js +++ b/server/routes/attachmentApi.js @@ -1,4 +1,5 @@ import { Meteor } from 'meteor/meteor'; +import { Accounts } from 'meteor/accounts-base'; import { WebApp } from 'meteor/webapp'; import { ReactiveCache } from '/imports/reactiveCache'; import { Attachments, fileStoreStrategyFactory } from '/models/attachments'; @@ -11,20 +12,24 @@ import { ObjectID } from 'bson'; // Attachment API HTTP routes if (Meteor.isServer) { - // Helper function to authenticate API requests + // Helper function to authenticate API requests using X-User-Id and X-Auth-Token function authenticateApiRequest(req) { - const authHeader = req.headers.authorization; - if (!authHeader || !authHeader.startsWith('Bearer ')) { - throw new Meteor.Error('unauthorized', 'Missing or invalid authorization header'); + const userId = req.headers['x-user-id']; + const authToken = req.headers['x-auth-token']; + + if (!userId || !authToken) { + throw new Meteor.Error('unauthorized', 'Missing X-User-Id or X-Auth-Token headers'); } - const token = authHeader.substring(7); - // Here you would validate the token and get the user ID - // For now, we'll use a simple approach - in production, you'd want proper JWT validation - const userId = token; // This should be replaced with proper token validation - - if (!userId) { - throw new Meteor.Error('unauthorized', 'Invalid token'); + // Hash the token and validate against stored login tokens + const hashedToken = Accounts._hashLoginToken(authToken); + const user = Meteor.users.findOne({ + _id: userId, + 'services.resume.loginTokens.hashedToken': hashedToken, + }); + + if (!user) { + throw new Meteor.Error('unauthorized', 'Invalid credentials'); } return userId; @@ -47,15 +52,33 @@ if (Meteor.isServer) { return next(); } + // Set timeout to prevent hanging connections + const timeout = setTimeout(() => { + if (!res.headersSent) { + sendErrorResponse(res, 408, 'Request timeout'); + } + }, 30000); // 30 second timeout + try { const userId = authenticateApiRequest(req); let body = ''; + let bodyComplete = false; + req.on('data', chunk => { body += chunk.toString(); + // Prevent excessive payload + if (body.length > 50 * 1024 * 1024) { // 50MB limit + req.connection.destroy(); + clearTimeout(timeout); + } }); req.on('end', () => { + if (bodyComplete) return; // Already processed + bodyComplete = true; + clearTimeout(timeout); + try { const data = JSON.parse(body); const { boardId, swimlaneId, listId, cardId, fileData, fileName, fileType, storageBackend } = data; @@ -154,7 +177,16 @@ if (Meteor.isServer) { sendErrorResponse(res, 500, error.message); } }); + + req.on('error', (error) => { + clearTimeout(timeout); + if (!res.headersSent) { + console.error('Request error:', error); + sendErrorResponse(res, 400, 'Request error'); + } + }); } catch (error) { + clearTimeout(timeout); sendErrorResponse(res, 401, error.message); } }); @@ -287,15 +319,31 @@ if (Meteor.isServer) { return next(); } + const timeout = setTimeout(() => { + if (!res.headersSent) { + sendErrorResponse(res, 408, 'Request timeout'); + } + }, 30000); + try { const userId = authenticateApiRequest(req); let body = ''; + let bodyComplete = false; + req.on('data', chunk => { body += chunk.toString(); + if (body.length > 10 * 1024 * 1024) { // 10MB limit for metadata + req.connection.destroy(); + clearTimeout(timeout); + } }); req.on('end', () => { + if (bodyComplete) return; + bodyComplete = true; + clearTimeout(timeout); + try { const data = JSON.parse(body); const { attachmentId, targetBoardId, targetSwimlaneId, targetListId, targetCardId } = data; @@ -388,7 +436,16 @@ if (Meteor.isServer) { sendErrorResponse(res, 500, error.message); } }); + + req.on('error', (error) => { + clearTimeout(timeout); + if (!res.headersSent) { + console.error('Request error:', error); + sendErrorResponse(res, 400, 'Request error'); + } + }); } catch (error) { + clearTimeout(timeout); sendErrorResponse(res, 401, error.message); } }); @@ -399,15 +456,31 @@ if (Meteor.isServer) { return next(); } + const timeout = setTimeout(() => { + if (!res.headersSent) { + sendErrorResponse(res, 408, 'Request timeout'); + } + }, 30000); + try { const userId = authenticateApiRequest(req); let body = ''; + let bodyComplete = false; + req.on('data', chunk => { body += chunk.toString(); + if (body.length > 10 * 1024 * 1024) { + req.connection.destroy(); + clearTimeout(timeout); + } }); req.on('end', () => { + if (bodyComplete) return; + bodyComplete = true; + clearTimeout(timeout); + try { const data = JSON.parse(body); const { attachmentId, targetBoardId, targetSwimlaneId, targetListId, targetCardId } = data; @@ -461,7 +534,16 @@ if (Meteor.isServer) { sendErrorResponse(res, 500, error.message); } }); + + req.on('error', (error) => { + clearTimeout(timeout); + if (!res.headersSent) { + console.error('Request error:', error); + sendErrorResponse(res, 400, 'Request error'); + } + }); } catch (error) { + clearTimeout(timeout); sendErrorResponse(res, 401, error.message); } });