Merge pull request #7386 from Countly/hook-valid

[SER-2844] Implement domain/ip address validation for hooks with http effect

Author: ar2rsawseen

CHANGELOG.md

@@ -1,3 +1,7 @@
+## Version 25.03.X
+Fixes:
+- [hooks] Implement domain/ip address validation for hooks with http effect
+
 ## Version 25.03.39
 Fixes:
 - [core] Fixed replaceDatabaseString incorrectly replacing "countly" in the MongoDB username when it appears in the connection URL

plugins/hooks/api/api.js

@@ -9,6 +9,7 @@ const log = common.log('hooks:api');
 const _ = require('lodash');
 const utils = require('./utils');
 const rights = require('../../../api/utils/rights');
+const ssrfProtection = require('./ssrf-protection');
 
 const FEATURE_NAME = 'hooks';
 
@@ -232,7 +233,7 @@ const CheckEffectProperties = function(effect) {
     //todo: add more validation for effect types
     if (effect) {
         if (effect.type === "HTTPEffect") {
-            rules.url = { 'required': true, 'type': 'URL', 'regex': '^(?!.*(?:localhost|127\\.0\\.0\\.1|\\[::1\\])).*(?:https?|ftp):\\/\\/(?:[a-zA-Z0-9-]+(?:\\.[a-zA-Z0-9-]+)+|\\[(?:[a-fA-F0-9]{1,4}:){7}[a-fA-F0-9]{1,4}\\])(?::\\d{1,5})?(?:\\/\\S*)?$' };
+            rules.url = { 'required': true, 'type': 'URL' };
             rules.headers = { 'required': false, 'type': 'Object' };
         }
     }
@@ -273,7 +274,7 @@ plugins.register("/permissions/features", function(ob) {
 plugins.register("/i/hook/save", function(ob) {
     let paramsInstance = ob.params;
 
-    validateCreate(ob.params, FEATURE_NAME, function(params) {
+    validateCreate(ob.params, FEATURE_NAME, async function(params) {
         let hookConfig = params.qstring.hook_config;
         if (!hookConfig) {
             common.returnMessage(params, 400, 'Invalid hookConfig');
@@ -290,9 +291,12 @@ plugins.register("/i/hook/save", function(ob) {
                     return true;
                 }
 
-                if (hookConfig.effects && !validateEffects(hookConfig.effects)) {
-                    common.returnMessage(params, 400, 'Invalid configuration for effects');
-                    return true;
+                if (hookConfig.effects) {
+                    const effectValidation = await validateEffects(hookConfig.effects);
+                    if (!effectValidation.valid) {
+                        common.returnMessage(params, 400, effectValidation.error || 'Invalid configuration for effects');
+                        return true;
+                    }
                 }
 
                 if (hookConfig._id) {
@@ -367,20 +371,25 @@ plugins.register("/i/hook/save", function(ob) {
 
 /***
  * @param {array} effects - array of effects
- * @returns {boolean} isValid - true if all effects are valid
  */
-function validateEffects(effects) {
-    let isValid = true;
+async function validateEffects(effects) {
     if (effects) {
         for (let i = 0; i < effects.length; i++) {
             if (!(common.validateArgs(effects[i].configuration, CheckEffectProperties(effects[i])))) {
-                isValid = false;
-                break;
+                return { valid: false, error: 'Invalid effect configuration' };
+            }
+
+            // SSRF protection: validate HTTPEffect URLs with DNS resolution
+            if (effects[i].type === "HTTPEffect" && effects[i].configuration && effects[i].configuration.url) {
+                const urlCheck = await ssrfProtection.isUrlSafe(effects[i].configuration.url);
+                if (!urlCheck.safe) {
+                    return { valid: false, error: 'Unsafe URL in HTTPEffect: ' + urlCheck.error };
+                }
             }
         }
     }
 
-    return isValid;
+    return { valid: true, error: null };
 
 }
 
@@ -636,9 +645,12 @@ plugins.register("/i/hook/test", function(ob) {
             }
 
             // Null check for effects
-            if (hookConfig.effects && !validateEffects(hookConfig.effects)) {
-                common.returnMessage(params, 400, 'Config invalid');
-                return; // Add return to exit early
+            if (hookConfig.effects) {
+                const effectValidation = await validateEffects(hookConfig.effects);
+                if (!effectValidation.valid) {
+                    common.returnMessage(params, 400, effectValidation.error || 'Config invalid');
+                    return; // Add return to exit early
+                }
             }
 
 

plugins/hooks/api/parts/effects/http.js

@@ -2,6 +2,7 @@ const plugins = require("../../../../pluginManager.js");
 const request = require("countly-request")(plugins.getConfig("security"));
 const utils = require("../../utils");
 const common = require('../../../../../api/utils/common.js');
+const ssrfProtection = require('../../ssrf-protection');
 const log = common.log("hooks:api:api_endpoint_trigger");
 
 /**
@@ -47,17 +48,39 @@ class HTTPEffect {
             const parsedRequestData = utils.parseStringTemplate(requestData, params, method);
             log.d("[hook http effect ]", parsedURL, parsedRequestData, method);
 
-            // todo: assemble params for request;
-            // const params = {}
+            // Revalidate URL after template expansion.
+            // The URL was validated at save time, but template variables
+            // (e.g. {{path}}) may have been replaced with malicious values
+            // that redirect to internal/private addresses.
+            const urlCheck = await ssrfProtection.isUrlSafe(parsedURL);
+            if (!urlCheck.safe) {
+                const ssrfError = new Error('SSRF protection: blocked HTTP effect — ' + urlCheck.error);
+                logs.push(`Error: ${ssrfError.message}`);
+                utils.addErrorRecord(rule._id, ssrfError, params, effectStep, _originalInput);
+                log.e("[hook http effect ] SSRF blocked:", urlCheck.error);
+                return {...options, logs};
+            }
+
             const requestHeaders = headers || {};
             const methodOption = method && method.toLowerCase() || "get";
             switch (methodOption) {
-            case 'get':
-                await request.get({
-                    uri: parsedURL + "?" + parsedRequestData,
+            case 'get': {
+                // For GET, also validate the full URL with query string
+                const fullGetUrl = parsedURL + "?" + parsedRequestData;
+                const getUrlCheck = await ssrfProtection.isUrlSafe(fullGetUrl);
+                if (!getUrlCheck.safe) {
+                    const ssrfError = new Error('SSRF protection: blocked GET URL — ' + getUrlCheck.error);
+                    logs.push(`Error: ${ssrfError.message}`);
+                    utils.addErrorRecord(rule._id, ssrfError, params, effectStep, _originalInput);
+                    log.e("[hook http effect ] SSRF blocked:", getUrlCheck.error);
+                    return {...options, logs};
+                }
+
+                await request.get(ssrfProtection.getSsrfSafeOptions({
+                    uri: fullGetUrl,
                     timeout: this._timeout,
-                    headers: requestHeaders
-                }, function(e, r, body) {
+                    headers: requestHeaders,
+                }), function(e, r, body) {
                     log.d("[http get effect]", e, body);
                     if (e) {
                         logs.push(`Error: ${e.message}`);
@@ -66,6 +89,7 @@ class HTTPEffect {
                     }
                 });
                 break;
+            }
             case 'post': {
                 //support post formData
                 let parsedJSON = {};
@@ -90,13 +114,13 @@ class HTTPEffect {
                     utils.addErrorRecord(rule._id, e, params, effectStep, _originalInput);
                 }
                 if (Object.keys(parsedJSON).length) {
-                    await request({
+                    await request(ssrfProtection.getSsrfSafeOptions({
                         method: 'POST',
                         uri: parsedURL,
                         json: parsedJSON,
                         timeout: this._timeout,
-                        headers: requestHeaders
-                    },
+                        headers: requestHeaders,
+                    }),
                     function(e, r, body) {
                         log.d("[httpeffects]", e, body, rule);
                         if (e) {