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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
40 changes: 26 additions & 14 deletions plugins/hooks/api/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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' };
}
}
Expand Down Expand Up @@ -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');
Expand All @@ -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) {
Expand Down Expand Up @@ -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 };

}

Expand Down Expand Up @@ -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
}
}


Expand Down
44 changes: 34 additions & 10 deletions plugins/hooks/api/parts/effects/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");

/**
Expand Down Expand Up @@ -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}`);
Expand All @@ -66,6 +89,7 @@ class HTTPEffect {
}
});
break;
}
case 'post': {
//support post formData
let parsedJSON = {};
Expand All @@ -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) {
Expand Down
Loading
Loading