From 34df9924537fd06f2d172d4f66386fc0ac993e55 Mon Sep 17 00:00:00 2001 From: Gunnar Date: Tue, 25 Jun 2024 11:22:57 +0200 Subject: [PATCH 1/2] Update RateLimiter.cfc, fix for CONTENTBOX-1512 --- .../models/security/RateLimiter.cfc | 113 +++++++----------- 1 file changed, 42 insertions(+), 71 deletions(-) diff --git a/modules/contentbox/models/security/RateLimiter.cfc b/modules/contentbox/models/security/RateLimiter.cfc index cd67957d6..da886728b 100755 --- a/modules/contentbox/models/security/RateLimiter.cfc +++ b/modules/contentbox/models/security/RateLimiter.cfc @@ -10,14 +10,7 @@ component extends="coldbox.system.Interceptor" { // DI property name="settingService" inject="id:settingService@contentbox"; property name="securityService" inject="id:securityService@contentbox"; - - /** - * Configure - */ - function configure(){ - // the limiter data - variables.limitData = {}; - } + property name = "cachebox" inject = "Cachebox"; /** * Limiter @@ -42,99 +35,77 @@ component extends="coldbox.system.Interceptor" { } } - /** - * Written by Charlie Arehart, charlie@carehart.org, in 2009, updated 2012, modified by Luis Majano 2016 - * - Throttles requests made more than "count" times within "duration" seconds from single IP. - * - Duck typed for performance - * - * @count The throttle counter - * @duration The time in seconds to limit - * @event The request context object - * @settings The settings structure - */ - private function limiter( count, duration, event, settings ){ + /** + * Written by Charlie Arehart, charlie@carehart.org, in 2009, updated 2012, modified by Luis Majano 2016 + * - Throttles requests made more than "count" times within "duration" seconds from single IP. + * - Duck typed for performance + * @count The throttle counter + * @duration The time in seconds to limit + * @event The request context object + * @settings The settings structure + */ + private function limiter( count, duration, event, settings ) { // Get real IP address of requester - var realIP = variables.securityService.getRealIP(); + var realIP = variables.securityService.getRealIP(); + var cache = cachebox.getDefaultCache(); + var cacheKey = 'limiter'&realIP; // If first time visit, create record. - if ( !structKeyExists( variables.limitData, realIP ) ) { - lock name="cb-ratelimiter-#hash( realIP )#" type="exclusive" throwontimeout="true" timeout="5" { - if ( !structKeyExists( variables.limitData, realIP ) ) { - variables.limitData[ realIP ] = { attempts : 1, lastAttempt : now() }; - return this; - } - } - } - - lock name="cb-ratelimiter-#hash( realIP )#" type="readonly" throwontimeout="true" timeout="5" { - var targetData = variables.limitData[ realIP ]; + var targetData = cache.get( cacheKey ); + if( isNull( targetData ) ){ + cache.set( cacheKey, { attempts = 1, lastAttempt = now() }); + return this; } log.debug( "Limit data", targetData ); - // log.info( "DateDiff " & dateDiff( "s", targetData.lastAttempt, Now() ) ); - // log.info( "Within Duration " & dateDiff( "s", targetData.lastAttempt, Now() ) LT arguments.duration ); - + //log.info( "DateDiff " & dateDiff( "s", targetData.lastAttempt, Now() ) ); + //log.info( "Within Duration " & dateDiff( "s", targetData.lastAttempt, Now() ) LT arguments.duration ); + // Are we executing another request withing our duration in seconds? Ex: Has X seconds passed before last attempt - if ( dateDiff( "s", targetData.lastAttempt, now() ) LT arguments.duration ) { + if( dateDiff( "s", targetData.lastAttempt, Now() ) LT arguments.duration ){ // Limit by count? - if ( targetData.attempts GT arguments.count ) { - if ( settings.cb_security_rate_limiter_logging && log.canInfo() ) { + if( targetData.attempts GT arguments.count ){ + + if( settings.cb_security_rate_limiter_logging && log.canInfo() ){ // Log it to app logs - log.info( - "'limiter invoked for:','#realIP#',#targetData.attempts#,#cgi.request_method#,'#cgi.SCRIPT_NAME#', '#cgi.QUERY_STRING#','#cgi.http_user_agent#','#targetData.lastAttempt#',#listLen( cgi.http_cookie, ";" )#" - ); + log.info( "'limiter invoked for:','#realIP#',#targetData.attempts#,#cgi.request_method#,'#cgi.SCRIPT_NAME#', '#cgi.QUERY_STRING#','#cgi.http_user_agent#','#targetData.lastAttempt#',#listlen(cgi.http_cookie,";" )#" ); } - + // Log attempt - lock name="cb-ratelimiter-#hash( realIP )#" type="exclusive" throwontimeout="true" timeout="5" { - targetData.attempts++; - targetData.lastAttempt = now(); - } + targetData.attempts++; + targetData.lastAttempt = now(); + cache.set( cacheKey, targetData ); // Do we have a redirect URL setup? - if ( len( settings.cb_security_rate_limiter_redirectURL ) ) { - location( - settings.cb_security_rate_limiter_redirectURL, - "false", - "301" - ); + if( len( settings.cb_security_rate_limiter_redirectURL ) ){ + location( settings.cb_security_rate_limiter_redirectURL, "false", "301" ); return; } // Output Message - writeOutput( - replaceNoCase( - settings[ "cb_security_rate_limiter_message" ], - "{duration}", - arguments.duration, - "all" - ) + writeOutput( + replaceNoCase( settings[ "cb_security_rate_limiter_message" ], "{duration}", arguments.duration, "all" ) ); - arguments.event - .setHTTPHeader( statusCode = "503", statusText = "Service Unavailable" ) - .setHTTPHeader( name = "Retry-After", value = arguments.duration ); + arguments.event.setHTTPHeader( statusCode="503", statusText="Service Unavailable" ) + .setHTTPHeader( name="Retry-After", value=arguments.duration ); // No execution anymore. event.noExecution(); // Hard abort; - abort; + abort; } else { // Log attempt - lock name="cb-ratelimiter-#hash( realIP )#" type="exclusive" throwontimeout="true" timeout="5" { - targetData.attempts++; - targetData.lastAttempt = now(); - } + targetData.attempts++; + targetData.lastAttempt = now(); } } else { // Reset attempts counter, since we are in the safe zone, just log the last attempt timestamp - lock name="cb-ratelimiter-#hash( realIP )#" type="exclusive" throwontimeout="true" timeout="5" { - targetData.attempts = 0; - targetData.lastAttempt = now(); - } + targetData.attempts = 0; + targetData.lastAttempt = now(); } + cache.set( cacheKey, targetData ); return this; } - } From 447398c29f6ed8cef8b567a3e508e3a6f1a162db Mon Sep 17 00:00:00 2001 From: Gunnar Date: Wed, 26 Jun 2024 15:31:16 +0200 Subject: [PATCH 2/2] Update RateLimiter.cfc use cache.getOrSet, a little bit of formatting --- .../models/security/RateLimiter.cfc | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/modules/contentbox/models/security/RateLimiter.cfc b/modules/contentbox/models/security/RateLimiter.cfc index da886728b..48c005d00 100755 --- a/modules/contentbox/models/security/RateLimiter.cfc +++ b/modules/contentbox/models/security/RateLimiter.cfc @@ -8,12 +8,13 @@ component extends="coldbox.system.Interceptor" { // DI - property name="settingService" inject="id:settingService@contentbox"; - property name="securityService" inject="id:securityService@contentbox"; + property name = "settingService" inject = "id:settingService@contentbox"; + property name = "securityService" inject = "id:securityService@contentbox"; property name = "cachebox" inject = "Cachebox"; /** - * Limiter + * onRequestCapture + * fires before any event caching or processing */ function onRequestCapture( event, data, buffer ){ var allSettings = variables.settingService.getAllSettings(); @@ -50,11 +51,15 @@ component extends="coldbox.system.Interceptor" { var cache = cachebox.getDefaultCache(); var cacheKey = 'limiter'&realIP; - // If first time visit, create record. - var targetData = cache.get( cacheKey ); - if( isNull( targetData ) ){ - cache.set( cacheKey, { attempts = 1, lastAttempt = now() }); - return this; + var targetData = cache.getOrSet( cacheKey, function(){ + return { attempts = 0, lastAttempt = now() } + } ); + + // on first visit no further processing + if( targetData.attempts == 0 ){ + targetData.attempts++; + cache.set( cacheKey, targetData ); + return this; } log.debug( "Limit data", targetData );