diff --git a/system/web/context/RequestContext.cfc b/system/web/context/RequestContext.cfc index 962bc7d26..8dab22a81 100644 --- a/system/web/context/RequestContext.cfc +++ b/system/web/context/RequestContext.cfc @@ -1799,10 +1799,34 @@ component serializable="false" accessors="true" { } /** - * Get the HTTP Request Method Type + * Get the original (transport-level) HTTP Request Method Type, ignoring any _method override. + */ + string function getOriginalHTTPMethod(){ + return uCase( CGI.REQUEST_METHOD ); + } + + /** + * Get the effective HTTP Request Method Type. + * Method spoofing via the _method parameter is only honored when the original + * transport-level request method is POST, and only for PUT, PATCH, and DELETE overrides. + * This prevents GET requests from spoofing destructive HTTP methods. */ string function getHTTPMethod(){ - return getValue( "_method", CGI.REQUEST_METHOD ); + var originalMethod = getOriginalHTTPMethod(); + + // Only honor _method override on POST requests + if ( originalMethod != "POST" ) { + return originalMethod; + } + + var overriddenMethod = uCase( trim( getValue( "_method", "" ) ) ); + + // Only allow overriding to PUT, PATCH, or DELETE from POST + if ( listFindNoCase( "PUT,PATCH,DELETE", overriddenMethod ) ) { + return overriddenMethod; + } + + return originalMethod; } /** diff --git a/tests/specs/web/context/RequestContextTest.cfc b/tests/specs/web/context/RequestContextTest.cfc index b52004f1e..bbbcc6b56 100755 --- a/tests/specs/web/context/RequestContextTest.cfc +++ b/tests/specs/web/context/RequestContextTest.cfc @@ -856,4 +856,56 @@ component extends="coldbox.system.testing.BaseModelTest" { expect( event.getSesBaseUrl() ).toInclude( event.getSESBasePath() ); } + /** + * Tests for method spoofing security fix (COLDBOX-1406). + * _method override should only be honored when the original request is POST. + */ + function testGetHTTPMethodNoOverrideOnGET(){ + var event = getRequestContext().$( "getOriginalHTTPMethod", "GET" ); + event.setValue( "_method", "DELETE" ); + expect( event.getHTTPMethod() ).toBe( "GET" ); + } + + function testGetHTTPMethodNoOverrideOnHEAD(){ + var event = getRequestContext().$( "getOriginalHTTPMethod", "HEAD" ); + event.setValue( "_method", "DELETE" ); + expect( event.getHTTPMethod() ).toBe( "HEAD" ); + } + + function testGetHTTPMethodNoOverrideOnPUT(){ + var event = getRequestContext().$( "getOriginalHTTPMethod", "PUT" ); + event.setValue( "_method", "DELETE" ); + expect( event.getHTTPMethod() ).toBe( "PUT" ); + } + + function testGetHTTPMethodPostDeleteOverride(){ + var event = getRequestContext().$( "getOriginalHTTPMethod", "POST" ); + event.setValue( "_method", "DELETE" ); + expect( event.getHTTPMethod() ).toBe( "DELETE" ); + } + + function testGetHTTPMethodPostPutOverride(){ + var event = getRequestContext().$( "getOriginalHTTPMethod", "POST" ); + event.setValue( "_method", "PUT" ); + expect( event.getHTTPMethod() ).toBe( "PUT" ); + } + + function testGetHTTPMethodPostPatchOverride(){ + var event = getRequestContext().$( "getOriginalHTTPMethod", "POST" ); + event.setValue( "_method", "PATCH" ); + expect( event.getHTTPMethod() ).toBe( "PATCH" ); + } + + function testGetHTTPMethodPostWithNoOverride(){ + var event = getRequestContext().$( "getOriginalHTTPMethod", "POST" ); + expect( event.getHTTPMethod() ).toBe( "POST" ); + } + + function testGetHTTPMethodPostGetOverrideIgnored(){ + // POST _method=GET should not be honored; result stays POST + var event = getRequestContext().$( "getOriginalHTTPMethod", "POST" ); + event.setValue( "_method", "GET" ); + expect( event.getHTTPMethod() ).toBe( "POST" ); + } + }