diff --git a/models/integrations.js b/models/integrations.js index d3c7aee5b..174b8d905 100644 --- a/models/integrations.js +++ b/models/integrations.js @@ -36,11 +36,45 @@ Integrations.attachSchema( defaultValue: ['all'], }, url: { - // URL validation regex (https://mathiasbynens.be/demo/url-regex) + // URL validation with SSRF protection /** * URL validation regex (https://mathiasbynens.be/demo/url-regex) + * Includes validation to block private/loopback addresses and ensure safe protocols */ type: String, + custom() { + try { + const u = new URL(this.value); + + // Only allow http and https protocols + if (!['http:', 'https:'].includes(u.protocol)) { + return 'invalidProtocol'; + } + + // Block private/loopback IP ranges and hostnames + const hostname = u.hostname.toLowerCase(); + const blockedPatterns = [ + /^127\./, // 127.x.x.x (loopback) + /^10\./, // 10.x.x.x (private) + /^172\.(1[6-9]|2\d|3[01])\./, // 172.16-31.x.x (private) + /^192\.168\./, // 192.168.x.x (private) + /^0\./, // 0.x.x.x (current network) + /^::1$/, // IPv6 loopback + /^fe80:/, // IPv6 link-local + /^fc00:/, // IPv6 unique local + /^fd00:/, // IPv6 unique local + /^localhost$/i, + /\.local$/i, + /^169\.254\./, // link-local IP + ]; + + if (blockedPatterns.some(pattern => pattern.test(hostname))) { + return 'privateAddress'; + } + } catch { + return 'invalidUrl'; + } + }, }, token: { /** @@ -198,13 +232,13 @@ if (Meteor.isServer) { * @param {string} url the URL of the integration * @return_type {_id: string} */ - JsonRoutes.add('POST', '/api/boards/:boardId/integrations', function( + JsonRoutes.add('POST', '/api/boards/:boardId/integrations', async function( req, res, ) { try { const paramBoardId = req.params.boardId; - Authentication.checkBoardAccess(req.userId, paramBoardId); + await Authentication.checkBoardAdmin(req.userId, paramBoardId); const id = Integrations.insert({ userId: req.userId, @@ -239,14 +273,14 @@ if (Meteor.isServer) { * @param {string} [activities] new list of activities of the integration * @return_type {_id: string} */ - JsonRoutes.add('PUT', '/api/boards/:boardId/integrations/:intId', function( + JsonRoutes.add('PUT', '/api/boards/:boardId/integrations/:intId', async function( req, res, ) { try { const paramBoardId = req.params.boardId; const paramIntId = req.params.intId; - Authentication.checkBoardAccess(req.userId, paramBoardId); + await Authentication.checkBoardAdmin(req.userId, paramBoardId); if (req.body.hasOwnProperty('enabled')) { const newEnabled = req.body.enabled; @@ -315,7 +349,7 @@ if (Meteor.isServer) { const paramBoardId = req.params.boardId; const paramIntId = req.params.intId; const newActivities = req.body.activities; - Authentication.checkBoardAccess(req.userId, paramBoardId); + await Authentication.checkBoardAdmin(req.userId, paramBoardId); Integrations.direct.update( { _id: paramIntId, boardId: paramBoardId }, @@ -355,7 +389,7 @@ if (Meteor.isServer) { const paramBoardId = req.params.boardId; const paramIntId = req.params.intId; const newActivities = req.body.activities; - Authentication.checkBoardAccess(req.userId, paramBoardId); + await Authentication.checkBoardAdmin(req.userId, paramBoardId); Integrations.direct.update( { _id: paramIntId, boardId: paramBoardId }, @@ -386,14 +420,14 @@ if (Meteor.isServer) { * @param {string} intId the integration ID * @return_type {_id: string} */ - JsonRoutes.add('DELETE', '/api/boards/:boardId/integrations/:intId', function( + JsonRoutes.add('DELETE', '/api/boards/:boardId/integrations/:intId', async function( req, res, ) { try { const paramBoardId = req.params.boardId; const paramIntId = req.params.intId; - Authentication.checkBoardAccess(req.userId, paramBoardId); + await Authentication.checkBoardAdmin(req.userId, paramBoardId); Integrations.direct.remove({ _id: paramIntId, boardId: paramBoardId }); JsonRoutes.sendResult(res, { diff --git a/server/authentication.js b/server/authentication.js index 77aac7f9f..9309cd975 100644 --- a/server/authentication.js +++ b/server/authentication.js @@ -68,6 +68,14 @@ Meteor.startup(() => { await Authentication.checkAdminOrCondition(userId, writeAccess); }; + // Helper function. Will throw an error if the user is not a board admin. + Authentication.checkBoardAdmin = async function(userId, boardId) { + Authentication.checkLoggedIn(userId); + const board = await ReactiveCache.getBoard(boardId); + const adminAccess = board.members.some(e => e.userId === userId && e.isActive && e.isAdmin); + await Authentication.checkAdminOrCondition(userId, adminAccess); + }; + if (Meteor.isServer) { if ( process.env.ORACLE_OIM_ENABLED === 'true' || diff --git a/server/notifications/outgoing.js b/server/notifications/outgoing.js index 1703b1ead..afabf549c 100644 --- a/server/notifications/outgoing.js +++ b/server/notifications/outgoing.js @@ -12,6 +12,43 @@ if (Meteor.isServer) { }); }); + // SSRF Protection: Validate webhook URLs before posting + const isValidWebhookUrl = (urlString) => { + try { + const u = new URL(urlString); + + // Only allow http and https protocols + if (!['http:', 'https:'].includes(u.protocol)) { + return false; + } + + // Block private/loopback IP ranges and hostnames + const hostname = u.hostname.toLowerCase(); + const blockedPatterns = [ + /^127\./, // 127.x.x.x (loopback) + /^10\./, // 10.x.x.x (private) + /^172\.(1[6-9]|2\d|3[01])\./, // 172.16-31.x.x (private) + /^192\.168\./, // 192.168.x.x (private) + /^0\./, // 0.x.x.x (current network) + /^::1$/, // IPv6 loopback + /^fe80:/, // IPv6 link-local + /^fc00:/, // IPv6 unique local + /^fd00:/, // IPv6 unique local + /^localhost$/i, + /\.local$/i, + /^169\.254\./, // link-local IP (AWS metadata) + ]; + + if (blockedPatterns.some(pattern => pattern.test(hostname))) { + return false; + } + + return true; + } catch { + return false; + } + }; + const Lock = { _lock: {}, _timer: {}, @@ -70,13 +107,24 @@ if (Meteor.isServer) { 'label', 'attachmentId', ]; - const responseFunc = async data => { + const responseFunc = async (data, integration) => { const paramCommentId = data.commentId; const paramCardId = data.cardId; const paramBoardId = data.boardId; const newComment = data.comment; - if (paramCardId && paramBoardId && newComment) { - // only process data with the cardid, boardid and comment text, TODO can expand other functions here to react on returned data + + // Authorization: Verify the request is from a bidirectional webhook + if (!integration || integration.type !== Integrations.Const.TWOWAY) { + return; // Only bidirectional webhooks can update comments + } + + // Authorization: Prevent cross-board comment injection + if (paramBoardId !== integration.boardId) { + return; // Webhook can only modify comments in its own board + } + + if (paramCardId && paramBoardId && newComment && paramCommentId) { + // only process data with the commentId, cardId, boardId and comment text const comment = await ReactiveCache.getCardComment({ _id: paramCommentId, cardId: paramCardId, @@ -84,26 +132,15 @@ if (Meteor.isServer) { }); const board = await ReactiveCache.getBoard(paramBoardId); const card = await ReactiveCache.getCard(paramCardId); - if (board && card) { - if (comment) { - Lock.set(comment._id, newComment); - CardComments.direct.update(comment._id, { - $set: { - text: newComment, - }, - }); - } - } else { - const userId = data.userId; - if (userId) { - const inserted = CardComments.direct.insert({ + + if (board && card && comment) { + // Only update existing comments - do not create new comments from webhook responses + Lock.set(comment._id, newComment); + CardComments.direct.update(comment._id, { + $set: { text: newComment, - userId, - cardId, - boardId, - }); - Lock.set(inserted._id, newComment); - } + }, + }); } } }; @@ -175,6 +212,11 @@ if (Meteor.isServer) { const url = integration.url; + // SSRF Protection: Validate webhook URL before posting + if (!isValidWebhookUrl(url)) { + throw new Meteor.Error('invalid-webhook-url', 'Webhook URL is invalid or points to a private/internal address'); + } + if (is2way) { const cid = params.commentId; const comment = params.comment; @@ -198,7 +240,7 @@ if (Meteor.isServer) { const data = response.data; // only an JSON encoded response will be actioned if (data) { try { - await responseFunc(data); + await responseFunc(data, integration); } catch (e) { throw new Meteor.Error('error-process-data'); }