Просмотр исходного кода

Merge pull request #9952 from weseek/support/prevent-ssrf-for-slack-integration

support: Prevent SSRF for slack integration
mergify[bot] 10 месяцев назад
Родитель
Сommit
4f6227fdfc

+ 19 - 9
apps/app/src/server/routes/apiv3/slack-integration.js

@@ -5,25 +5,25 @@ import { InvalidGrowiCommandError } from '@growi/slack/dist/models';
 import { markdownSectionBlock } from '@growi/slack/dist/utils/block-kit-builder';
 import { markdownSectionBlock } from '@growi/slack/dist/utils/block-kit-builder';
 import { InteractionPayloadAccessor } from '@growi/slack/dist/utils/interaction-payload-accessor';
 import { InteractionPayloadAccessor } from '@growi/slack/dist/utils/interaction-payload-accessor';
 import { generateRespondUtil } from '@growi/slack/dist/utils/respond-util-factory';
 import { generateRespondUtil } from '@growi/slack/dist/utils/respond-util-factory';
+import { isValidResponseUrl } from '@growi/slack/dist/utils/response-url-validator';
 import { parseSlashCommand } from '@growi/slack/dist/utils/slash-command-parser';
 import { parseSlashCommand } from '@growi/slack/dist/utils/slash-command-parser';
+import express from 'express';
+import { body } from 'express-validator';
 import createError from 'http-errors';
 import createError from 'http-errors';
+import mongoose from 'mongoose';
 
 
 import { SlackCommandHandlerError } from '~/server/models/vo/slack-command-handler-error';
 import { SlackCommandHandlerError } from '~/server/models/vo/slack-command-handler-error';
 import { configManager } from '~/server/service/config-manager';
 import { configManager } from '~/server/service/config-manager';
 import { growiInfoService } from '~/server/service/growi-info';
 import { growiInfoService } from '~/server/service/growi-info';
 import loggerFactory from '~/utils/logger';
 import loggerFactory from '~/utils/logger';
 
 
-
-const express = require('express');
-const { body } = require('express-validator');
-const mongoose = require('mongoose');
+import { handleError } from '../../service/slack-command-handler/error-handler';
+import { checkPermission } from '../../util/slack-integration';
 
 
 
 
 const logger = loggerFactory('growi:routes:apiv3:slack-integration');
 const logger = loggerFactory('growi:routes:apiv3:slack-integration');
 const router = express.Router();
 const router = express.Router();
 const SlackAppIntegration = mongoose.model('SlackAppIntegration');
 const SlackAppIntegration = mongoose.model('SlackAppIntegration');
-const { handleError } = require('../../service/slack-command-handler/error-handler');
-const { checkPermission } = require('../../util/slack-integration');
 
 
 /** @param {import('~/server/crowi').default} crowi Crowi instance */
 /** @param {import('~/server/crowi').default} crowi Crowi instance */
 module.exports = (crowi) => {
 module.exports = (crowi) => {
@@ -221,7 +221,7 @@ module.exports = (crowi) => {
       throw SlackCommandHandlerError('App site url must exist.');
       throw SlackCommandHandlerError('App site url must exist.');
     }
     }
 
 
-    return generateRespondUtil(responseUrl, proxyUri, appSiteUrl);
+    return generateRespondUtil({ responseUrl, proxyUri, appSiteUrl });
   }
   }
 
 
   function getGrowiCommand(body) {
   function getGrowiCommand(body) {
@@ -286,6 +286,7 @@ module.exports = (crowi) => {
       await slackIntegrationService.handleCommandRequest(growiCommand, client, body, respondUtil);
       await slackIntegrationService.handleCommandRequest(growiCommand, client, body, respondUtil);
     }
     }
     catch (err) {
     catch (err) {
+      logger.error(err.message);
       return handleError(err, responseUrl);
       return handleError(err, responseUrl);
     }
     }
 
 
@@ -294,10 +295,18 @@ module.exports = (crowi) => {
   // TODO: this method will be a middleware when typescriptize in the future
   // TODO: this method will be a middleware when typescriptize in the future
   function getResponseUrl(req) {
   function getResponseUrl(req) {
     const { body } = req;
     const { body } = req;
-    const responseUrl = body?.growiCommand?.responseUrl;
+    const responseUrl = body?.growiCommand?.responseUrl ?? body.response_url;
+
     if (responseUrl == null) {
     if (responseUrl == null) {
-      return body.response_url; // may be null
+      return null;
+    }
+
+    const proxyUri = slackIntegrationService.proxyUriForCurrentType;
+
+    if (!isValidResponseUrl(responseUrl, proxyUri)) {
+      throw createError(400, 'Invalid response_url');
     }
     }
+
     return responseUrl;
     return responseUrl;
   }
   }
 
 
@@ -421,6 +430,7 @@ module.exports = (crowi) => {
       client = await slackIntegrationService.generateClientByTokenPtoG(tokenPtoG);
       client = await slackIntegrationService.generateClientByTokenPtoG(tokenPtoG);
     }
     }
     catch (err) {
     catch (err) {
+      logger.error(err.message);
       return handleError(err, responseUrl);
       return handleError(err, responseUrl);
     }
     }
 
 

+ 1 - 1
apps/slackbot-proxy/package.json

@@ -36,7 +36,7 @@
   "dependencies": {
   "dependencies": {
     "@godaddy/terminus": "^4.9.0",
     "@godaddy/terminus": "^4.9.0",
     "@growi/slack": "workspace:^",
     "@growi/slack": "workspace:^",
-    "@slack/oauth": "^2.0.1",
+    "@slack/oauth": "^3.0.3",
     "@slack/web-api": "^6.2.4",
     "@slack/web-api": "^6.2.4",
     "@tsed/common": "=6.43.0",
     "@tsed/common": "=6.43.0",
     "@tsed/di": "=6.43.0",
     "@tsed/di": "=6.43.0",

+ 1 - 0
apps/slackbot-proxy/src/services/InstallerService.ts

@@ -32,6 +32,7 @@ export class InstallerService {
       clientId,
       clientId,
       clientSecret,
       clientSecret,
       stateSecret,
       stateSecret,
+      legacyStateVerification: true,
       installationStore: {
       installationStore: {
         // upsert
         // upsert
         storeInstallation: async(slackInstallation: SlackInstallation<'v1' | 'v2', boolean>) => {
         storeInstallation: async(slackInstallation: SlackInstallation<'v1' | 'v2', boolean>) => {

+ 2 - 2
biome.json

@@ -7,6 +7,7 @@
       "coverage/**",
       "coverage/**",
       "vite.config.ts.timestamp-*",
       "vite.config.ts.timestamp-*",
       "vite.server.config.ts.timestamp-*",
       "vite.server.config.ts.timestamp-*",
+      "vite.client.config.ts.timestamp-*",
       ".pnpm-store/**",
       ".pnpm-store/**",
       ".turbo/**",
       ".turbo/**",
       ".vscode/**",
       ".vscode/**",
@@ -17,7 +18,6 @@
       ".eslintrc.js",
       ".eslintrc.js",
       ".stylelintrc.json",
       ".stylelintrc.json",
       "package.json",
       "package.json",
-
       "./apps/**",
       "./apps/**",
       "./packages/core/**",
       "./packages/core/**",
       "./packages/core-styles/**",
       "./packages/core-styles/**",
@@ -51,4 +51,4 @@
       "quoteStyle": "single"
       "quoteStyle": "single"
     }
     }
   }
   }
-}
+}

+ 1 - 1
packages/slack/package.json

@@ -49,7 +49,7 @@
     "test": "vitest run --coverage"
     "test": "vitest run --coverage"
   },
   },
   "dependencies": {
   "dependencies": {
-    "@slack/oauth": "^2.0.1",
+    "@slack/oauth": "^3.0.3",
     "@slack/web-api": "^6.2.4",
     "@slack/web-api": "^6.2.4",
     "@types/bunyan": "^1.8.10",
     "@types/bunyan": "^1.8.10",
     "@types/http-errors": "^2.0.3",
     "@types/http-errors": "^2.0.3",

+ 25 - 13
packages/slack/src/utils/respond-util-factory.ts

@@ -3,6 +3,7 @@ import urljoin from 'url-join';
 
 
 import type { IRespondUtil } from '../interfaces/respond-util';
 import type { IRespondUtil } from '../interfaces/respond-util';
 import type { RespondBodyForResponseUrl } from '../interfaces/response-url';
 import type { RespondBodyForResponseUrl } from '../interfaces/response-url';
+import { isValidResponseUrl } from './response-url-validator';
 
 
 type AxiosOptions = {
 type AxiosOptions = {
   headers?: {
   headers?: {
@@ -14,22 +15,35 @@ function getResponseUrlForProxy(proxyUri: string, responseUrl: string): string {
   return urljoin(proxyUri, `/g2s/respond?response_url=${responseUrl}`);
   return urljoin(proxyUri, `/g2s/respond?response_url=${responseUrl}`);
 }
 }
 
 
-function getUrl(responseUrl: string, proxyUri: string | null): string {
-  return proxyUri == null
-    ? responseUrl
-    : getResponseUrlForProxy(proxyUri, responseUrl);
+function getUrl(responseUrl: string, proxyUri?: string): string {
+  const finalUrl =
+    proxyUri === undefined
+      ? responseUrl
+      : getResponseUrlForProxy(proxyUri, responseUrl);
+
+  if (!isValidResponseUrl(responseUrl, proxyUri)) {
+    throw new Error('Invalid final response URL');
+  }
+
+  return finalUrl;
 }
 }
 
 
+type RespondUtilConstructorArgs = {
+  responseUrl: string;
+  appSiteUrl: string;
+  proxyUri?: string;
+};
+
 export class RespondUtil implements IRespondUtil {
 export class RespondUtil implements IRespondUtil {
   url!: string;
   url!: string;
 
 
   options!: AxiosOptions;
   options!: AxiosOptions;
 
 
-  constructor(
-    responseUrl: string,
-    proxyUri: string | null,
-    appSiteUrl: string,
-  ) {
+  constructor({
+    responseUrl,
+    appSiteUrl,
+    proxyUri,
+  }: RespondUtilConstructorArgs) {
     this.url = getUrl(responseUrl, proxyUri);
     this.url = getUrl(responseUrl, proxyUri);
 
 
     this.options = {
     this.options = {
@@ -88,9 +102,7 @@ export class RespondUtil implements IRespondUtil {
 }
 }
 
 
 export function generateRespondUtil(
 export function generateRespondUtil(
-  responseUrl: string,
-  proxyUri: string | null,
-  appSiteUrl: string,
+  args: RespondUtilConstructorArgs,
 ): RespondUtil {
 ): RespondUtil {
-  return new RespondUtil(responseUrl, proxyUri, appSiteUrl);
+  return new RespondUtil(args);
 }
 }

+ 43 - 0
packages/slack/src/utils/response-url-validator.ts

@@ -0,0 +1,43 @@
+import { URL } from 'node:url';
+
+const ALLOWED_SLACK_HOST = 'hooks.slack.com';
+
+export function isValidResponseUrl(
+  responseUrl: string,
+  slackbotProxyUri?: string,
+): boolean {
+  try {
+    const parsedUrl = new URL(responseUrl);
+
+    // Case 1: Direct to Slack
+    if (
+      parsedUrl.protocol === 'https:' &&
+      parsedUrl.hostname === ALLOWED_SLACK_HOST
+    ) {
+      return true;
+    }
+
+    // Case 2: Via slackbot-proxy
+    if (slackbotProxyUri) {
+      const parsedProxyUri = new URL(slackbotProxyUri);
+
+      if (
+        (parsedUrl.protocol === 'http:' || parsedUrl.protocol === 'https:') &&
+        parsedUrl.hostname === parsedProxyUri.hostname &&
+        parsedUrl.pathname === '/g2s/respond'
+      ) {
+        const slackResponseUrlParam =
+          parsedUrl.searchParams.get('response_url');
+        if (slackResponseUrlParam) {
+          // Recursively validate the response_url parameter
+          return isValidResponseUrl(slackResponseUrlParam); // No proxy URI for the inner check
+        }
+      }
+    }
+
+    return false;
+  } catch (error) {
+    // Invalid URL format
+    return false;
+  }
+}

+ 59 - 18
pnpm-lock.yaml

@@ -1081,8 +1081,8 @@ importers:
         specifier: workspace:^
         specifier: workspace:^
         version: link:../../packages/slack
         version: link:../../packages/slack
       '@slack/oauth':
       '@slack/oauth':
-        specifier: ^2.0.1
-        version: 2.6.2
+        specifier: ^3.0.3
+        version: 3.0.3
       '@slack/web-api':
       '@slack/web-api':
         specifier: ^6.2.4
         specifier: ^6.2.4
         version: 6.12.0
         version: 6.12.0
@@ -1744,8 +1744,8 @@ importers:
   packages/slack:
   packages/slack:
     dependencies:
     dependencies:
       '@slack/oauth':
       '@slack/oauth':
-        specifier: ^2.0.1
-        version: 2.6.2
+        specifier: ^3.0.3
+        version: 3.0.3
       '@slack/web-api':
       '@slack/web-api':
         specifier: ^6.2.4
         specifier: ^6.2.4
         version: 6.12.0
         version: 6.12.0
@@ -4407,9 +4407,13 @@ packages:
     resolution: {integrity: sha512-DTuBFbqu4gGfajREEMrkq5jBhcnskinhr4+AnfJEk48zhVeEv3XnUKGIX98B74kxhYsIMfApGGySTn7V3b5yBA==}
     resolution: {integrity: sha512-DTuBFbqu4gGfajREEMrkq5jBhcnskinhr4+AnfJEk48zhVeEv3XnUKGIX98B74kxhYsIMfApGGySTn7V3b5yBA==}
     engines: {node: '>= 12.13.0', npm: '>= 6.12.0'}
     engines: {node: '>= 12.13.0', npm: '>= 6.12.0'}
 
 
-  '@slack/oauth@2.6.2':
-    resolution: {integrity: sha512-2R3MyB/R63hTRXzk5J6wcui59TBxXzhk+Uh2/Xu3Wp3O4pXg/BNucQhP/DQbL/ScVhLvFtMXirLrKi0Yo5gIVw==}
-    engines: {node: '>=12.13.0', npm: '>=6.12.0'}
+  '@slack/logger@4.0.0':
+    resolution: {integrity: sha512-Wz7QYfPAlG/DR+DfABddUZeNgoeY7d1J39OCR2jR+v7VBsB8ezulDK5szTnDDPDwLH5IWhLvXIHlCFZV7MSKgA==}
+    engines: {node: '>= 18', npm: '>= 8.6.0'}
+
+  '@slack/oauth@3.0.3':
+    resolution: {integrity: sha512-N3pLJPacZ57bqmD1HzHDmHe/CNsL9pESZXRw7pfv6QXJVRgufPIW84aRpAez2Xb0616RpGBYZW5dZH0Nbskwyg==}
+    engines: {node: '>=18', npm: '>=8.6.0'}
 
 
   '@slack/types@1.10.0':
   '@slack/types@1.10.0':
     resolution: {integrity: sha512-tA7GG7Tj479vojfV3AoxbckalA48aK6giGjNtgH6ihpLwTyHE3fIgRrvt8TWfLwW8X8dyu7vgmAsGLRG7hWWOg==}
     resolution: {integrity: sha512-tA7GG7Tj479vojfV3AoxbckalA48aK6giGjNtgH6ihpLwTyHE3fIgRrvt8TWfLwW8X8dyu7vgmAsGLRG7hWWOg==}
@@ -4423,6 +4427,10 @@ packages:
     resolution: {integrity: sha512-RPw6F8rWfGveGkZEJ4+4jUin5iazxRK2q3FpQDz/FvdgzC3nZmPyLx8WRzc6nh0w3MBjEbphNnp2VZksfhpBIQ==}
     resolution: {integrity: sha512-RPw6F8rWfGveGkZEJ4+4jUin5iazxRK2q3FpQDz/FvdgzC3nZmPyLx8WRzc6nh0w3MBjEbphNnp2VZksfhpBIQ==}
     engines: {node: '>= 12.13.0', npm: '>= 6.12.0'}
     engines: {node: '>= 12.13.0', npm: '>= 6.12.0'}
 
 
+  '@slack/web-api@7.9.1':
+    resolution: {integrity: sha512-qMcb1oWw3Y/KlUIVJhkI8+NcQXq1lNymwf+ewk93ggZsGd6iuz9ObQsOEbvlqlx1J+wd8DmIm3DORGKs0fcKdg==}
+    engines: {node: '>= 18', npm: '>= 8.6.0'}
+
   '@slack/webhook@6.1.0':
   '@slack/webhook@6.1.0':
     resolution: {integrity: sha512-7AYNISyAjn/lA/VDwZ307K5ft5DojXgBd3DRrGoFN8XxIwIyRALdFhxBiMgAqeJH8eWoktvNwLK24R9hREEqpA==}
     resolution: {integrity: sha512-7AYNISyAjn/lA/VDwZ307K5ft5DojXgBd3DRrGoFN8XxIwIyRALdFhxBiMgAqeJH8eWoktvNwLK24R9hREEqpA==}
     engines: {node: '>= 12.13.0', npm: '>= 6.12.0'}
     engines: {node: '>= 12.13.0', npm: '>= 6.12.0'}
@@ -5371,8 +5379,8 @@ packages:
   '@types/jsonfile@6.1.4':
   '@types/jsonfile@6.1.4':
     resolution: {integrity: sha512-D5qGUYwjvnNNextdU59/+fI+spnwtTFmyQP0h+PfIOSkNfpU6AOICUOkm4i0OnSk+NyjdPJrxCDro0sJsWlRpQ==}
     resolution: {integrity: sha512-D5qGUYwjvnNNextdU59/+fI+spnwtTFmyQP0h+PfIOSkNfpU6AOICUOkm4i0OnSk+NyjdPJrxCDro0sJsWlRpQ==}
 
 
-  '@types/jsonwebtoken@8.5.9':
-    resolution: {integrity: sha512-272FMnFGzAVMGtu9tkr29hRL6bZj4Zs1KZNeHLnKqAvp06tAIcarTMwOh8/8bz4FmKRcMxZhZNeUAQsNLoiPhg==}
+  '@types/jsonwebtoken@9.0.9':
+    resolution: {integrity: sha512-uoe+GxEuHbvy12OUQct2X9JenKM3qAscquYymuQN4fMWG9DBQtykrQEFcAbVACF7qaLw9BePSodUL0kquqBJpQ==}
 
 
   '@types/katex@0.16.7':
   '@types/katex@0.16.7':
     resolution: {integrity: sha512-HMwFiRujE5PjrgwHQ25+bsLJgowjGjm5Z8FVSf0N6PwgJrwxH0QxzHYDcKsTfV3wva0vzrpqMTJS2jXPr5BMEQ==}
     resolution: {integrity: sha512-HMwFiRujE5PjrgwHQ25+bsLJgowjGjm5Z8FVSf0N6PwgJrwxH0QxzHYDcKsTfV3wva0vzrpqMTJS2jXPr5BMEQ==}
@@ -6180,6 +6188,9 @@ packages:
   axios@1.7.9:
   axios@1.7.9:
     resolution: {integrity: sha512-LhLcE7Hbiryz8oMDdDptSrWowmB4Bl6RCt6sIJKpRB4XtVf0iEgewX3au/pJqm+Py1kCASkb/FFKjxQaLtxJvw==}
     resolution: {integrity: sha512-LhLcE7Hbiryz8oMDdDptSrWowmB4Bl6RCt6sIJKpRB4XtVf0iEgewX3au/pJqm+Py1kCASkb/FFKjxQaLtxJvw==}
 
 
+  axios@1.9.0:
+    resolution: {integrity: sha512-re4CqKTJaURpzbLHtIi6XpDv20/CnpXOtjRY5/CU32L8gU8ek9UIivcfvSWvmKEngmVbrUtPpdDwWDWL7DNHvg==}
+
   axobject-query@2.2.0:
   axobject-query@2.2.0:
     resolution: {integrity: sha512-Td525n+iPOOyUQIeBfcASuG6uJsDOITl7Mds5gFyerkWiX7qhUTdYUBlSgNMyVqtSJqwpt1kXGLdUt6SykLMRA==}
     resolution: {integrity: sha512-Td525n+iPOOyUQIeBfcASuG6uJsDOITl7Mds5gFyerkWiX7qhUTdYUBlSgNMyVqtSJqwpt1kXGLdUt6SykLMRA==}
 
 
@@ -18606,12 +18617,16 @@ snapshots:
     dependencies:
     dependencies:
       '@types/node': 22.13.14
       '@types/node': 22.13.14
 
 
-  '@slack/oauth@2.6.2':
+  '@slack/logger@4.0.0':
     dependencies:
     dependencies:
-      '@slack/logger': 3.0.0
-      '@slack/web-api': 6.12.0
-      '@types/jsonwebtoken': 8.5.9
-      '@types/node': 22.13.14
+      '@types/node': 22.14.0
+
+  '@slack/oauth@3.0.3':
+    dependencies:
+      '@slack/logger': 4.0.0
+      '@slack/web-api': 7.9.1
+      '@types/jsonwebtoken': 9.0.9
+      '@types/node': 22.14.0
       jsonwebtoken: 9.0.2
       jsonwebtoken: 9.0.2
       lodash.isstring: 4.0.1
       lodash.isstring: 4.0.1
     transitivePeerDependencies:
     transitivePeerDependencies:
@@ -18637,6 +18652,23 @@ snapshots:
     transitivePeerDependencies:
     transitivePeerDependencies:
       - debug
       - debug
 
 
+  '@slack/web-api@7.9.1':
+    dependencies:
+      '@slack/logger': 4.0.0
+      '@slack/types': 2.14.0
+      '@types/node': 22.14.0
+      '@types/retry': 0.12.0
+      axios: 1.9.0
+      eventemitter3: 5.0.1
+      form-data: 4.0.0
+      is-electron: 2.2.2
+      is-stream: 2.0.0
+      p-queue: 6.6.2
+      p-retry: 4.6.2
+      retry: 0.13.1
+    transitivePeerDependencies:
+      - debug
+
   '@slack/webhook@6.1.0':
   '@slack/webhook@6.1.0':
     dependencies:
     dependencies:
       '@slack/types': 1.10.0
       '@slack/types': 1.10.0
@@ -20168,7 +20200,7 @@ snapshots:
 
 
   '@types/graceful-fs@4.1.9':
   '@types/graceful-fs@4.1.9':
     dependencies:
     dependencies:
-      '@types/node': 22.13.14
+      '@types/node': 22.14.0
 
 
   '@types/hast@2.3.4':
   '@types/hast@2.3.4':
     dependencies:
     dependencies:
@@ -20219,9 +20251,10 @@ snapshots:
     dependencies:
     dependencies:
       '@types/node': 22.13.14
       '@types/node': 22.13.14
 
 
-  '@types/jsonwebtoken@8.5.9':
+  '@types/jsonwebtoken@9.0.9':
     dependencies:
     dependencies:
-      '@types/node': 22.13.14
+      '@types/ms': 0.7.31
+      '@types/node': 22.14.0
 
 
   '@types/katex@0.16.7': {}
   '@types/katex@0.16.7': {}
 
 
@@ -20411,7 +20444,7 @@ snapshots:
 
 
   '@types/yauzl@2.10.3':
   '@types/yauzl@2.10.3':
     dependencies:
     dependencies:
-      '@types/node': 22.13.14
+      '@types/node': 22.14.0
     optional: true
     optional: true
 
 
   '@types/zen-observable@0.8.3': {}
   '@types/zen-observable@0.8.3': {}
@@ -21266,6 +21299,14 @@ snapshots:
     transitivePeerDependencies:
     transitivePeerDependencies:
       - debug
       - debug
 
 
+  axios@1.9.0:
+    dependencies:
+      follow-redirects: 1.15.9(debug@4.4.0)
+      form-data: 4.0.0
+      proxy-from-env: 1.1.0
+    transitivePeerDependencies:
+      - debug
+
   axobject-query@2.2.0: {}
   axobject-query@2.2.0: {}
 
 
   b4a@1.6.6: {}
   b4a@1.6.6: {}