Skip to content

Commit c1fcae6

Browse files
bpamiribpamiri
committed
fix(security): validate the CSRF decrypt result instead of trusting that it did not throw
$decryptCsrfCookieValue() read a cookie by trying the configured algorithm and falling back to the legacy bare "AES" (ECB) only from its catch block. That treats "did not throw" as "decrypted correctly", which is not true of a wrong-mode decrypt. Decrypting an ECB ciphertext under AES/CBC/PKCS5Padding throws only when the trailing plaintext bytes fail padding validation. They pass by chance roughly 1 time in 256 — Decrypt() then returns garbage, the legacy fallback never runs, and a perfectly good legacy cookie reads as corrupted. AES/GCM/NoPadding is authenticated and does reliably throw, so this only ever affected the engines that fall back to CBC. Impact is a reliability wart in the legacy-cookie migration window, NOT a security hole: the caller checks IsJSON() and returns "", so it fails closed. Worth saying plainly rather than inflating. The fix checks the RESULT. This cookie's plaintext is always the JSON that $generateCookieAuthenticityToken() writes, so JSON-ness is an invariant of the cookie rather than an assumption about it, and a non-JSON result means the wrong algorithm was used — which is exactly when the legacy attempt should still run. The legacy result is only preferred if it too looks like the payload, so a genuinely corrupt cookie is reported exactly as before. Found by diffing compat-matrix legs on #3351: CsrfCookieCipherSpec's legacy-AES case failed on lucee6/sqlite. It exercises this path and its payload carries a CreateUUID(), so the coin flip differs per run — across 326 non-empty legs it failed once. 1/326 = 0.31% observed against ~0.39% theoretical for random PKCS5 padding validity. Testing that deterministically needed a way to reach "returned garbage without throwing" every time rather than 1 run in 256. Probed the three behaviours on this engine: AES/CBC/PKCS5Padding -> THREW ("Given final block not properly padded") AES/CBC/NoPadding -> RETURNED, not JSON <- same state, every time AES/ECB/NoPadding -> RETURNED, valid JSON (same mode, decrypts fine) so the spec forces AES/CBC/NoPadding. Red-first on a COLD server (a warm one silently reuses the old compiled CFC): 288 pass / 1 fail / 1 error, the failure being `Expected [false] to be true` on exactly this path. lucee7 + sqlite, full core suite: develop 2c98644 4755 pass / 0 fail / 0 error this branch 4757 pass / 0 fail / 0 error Exactly +2, the new specs. Closes #3361 Signed-off-by: Peter Amiri <peter@alurium.com>
1 parent 2c98644 commit c1fcae6

10 files changed

Lines changed: 526 additions & 10 deletions

‎changelog.d/3361-csrf-legacy-fallback.fixed.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- A CSRF cookie written under the legacy bare `AES` (ECB) default is now always read via the legacy fallback, instead of roughly 1 time in 256 being reported as corrupted. `$decryptCsrfCookieValue()` tried the configured algorithm and fell back to bare `AES` only from its `catch` block — treating "did not throw" as "decrypted correctly". Decrypting an ECB ciphertext under `AES/CBC/PKCS5Padding` throws only when the trailing plaintext bytes fail padding validation, and they pass by chance about 1 time in 256, so `Decrypt()` returned garbage and the fallback never ran. The decrypt result is now validated before it is accepted: this cookie's plaintext is always the JSON written by `$generateCookieAuthenticityToken()`, so a non-JSON result means the wrong algorithm was used and the legacy attempt still runs. `AES/GCM/NoPadding` is authenticated and always threw, so only the engines that fall back to CBC were affected. Fails closed either way — a genuinely corrupt cookie is still reported exactly as before (#3361)

‎public/testbox/system/stubs/139184C0543CCDB338AEFB643110CB89.cfm‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<cfscript>
2+
variables[ "closeSSEStream" ] = variables[ "tmp_closeSSEStream_139184C0543CCDB338AEFB643110CB89" ];
3+
this[ "closeSSEStream" ] = variables[ "tmp_closeSSEStream_139184C0543CCDB338AEFB643110CB89" ];
4+
5+
// Clean up
6+
structDelete( variables, "tmp_closeSSEStream_139184C0543CCDB338AEFB643110CB89" );
7+
structDelete( this, "tmp_closeSSEStream_139184C0543CCDB338AEFB643110CB89" );
8+
public void function tmp_closeSSEStream_139184C0543CCDB338AEFB643110CB89(
9+
10+
) output=true {
11+
12+
var results = this._mockResults;
13+
var resultsKey = "closeSSEStream";
14+
var resultsCounter = 0;
15+
var internalCounter = 0;
16+
var resultsLen = 0;
17+
var callbackLen = 0;
18+
var argsHashKey = resultsKey & "|" & this.mockBox.normalizeArguments( arguments );
19+
var fCallBack = "";
20+
21+
// If Method & argument Hash Results, switch the results struct
22+
if (structKeyExists( this._mockArgResults, argsHashKey) ) {
23+
// Check if it is a callback
24+
if (isStruct( this._mockArgResults[ argsHashKey ]) &&
25+
structKeyExists( this._mockArgResults[ argsHashKey ], "type" ) &&
26+
structKeyExists( this._mockArgResults[ argsHashKey ], "target" ) ) {
27+
fCallBack = this._mockArgResults[ argsHashKey ].target;
28+
} else {
29+
// switch context and key
30+
results = this._mockArgResults;
31+
resultsKey = argsHashKey;
32+
}
33+
}
34+
35+
// Get the statemachine counter
36+
if (isSimpleValue( fCallBack) ) {
37+
resultsLen = arrayLen( results[ resultsKey ] );
38+
}
39+
40+
// Get the callback counter, if it exists
41+
if (structKeyExists( this._mockCallbacks, resultsKey) ) {
42+
callbackLen = arrayLen( this._mockCallbacks[ resultsKey ] );
43+
}
44+
45+
// Log the Method Call
46+
this._mockMethodCallCounters[ listFirst( resultsKey, "|" ) ] = this._mockMethodCallCounters[ listFirst( resultsKey, "|" ) ] + 1;
47+
48+
// Get the CallCounter Reference
49+
internalCounter = this._mockMethodCallCounters[listFirst(resultsKey,"|")];
50+
arrayAppend( this._mockCallLoggers["closeSSEStream"], arguments );
51+
}
52+
</cfscript>

‎public/testbox/system/stubs/16CA394252B074478BEBD2B3A9C8EA82.cfm‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<cfscript>
2+
variables[ "sendSSEComment" ] = variables[ "tmp_sendSSEComment_16CA394252B074478BEBD2B3A9C8EA82" ];
3+
this[ "sendSSEComment" ] = variables[ "tmp_sendSSEComment_16CA394252B074478BEBD2B3A9C8EA82" ];
4+
5+
// Clean up
6+
structDelete( variables, "tmp_sendSSEComment_16CA394252B074478BEBD2B3A9C8EA82" );
7+
structDelete( this, "tmp_sendSSEComment_16CA394252B074478BEBD2B3A9C8EA82" );
8+
public void function tmp_sendSSEComment_16CA394252B074478BEBD2B3A9C8EA82(
9+
10+
) output=true {
11+
12+
var results = this._mockResults;
13+
var resultsKey = "sendSSEComment";
14+
var resultsCounter = 0;
15+
var internalCounter = 0;
16+
var resultsLen = 0;
17+
var callbackLen = 0;
18+
var argsHashKey = resultsKey & "|" & this.mockBox.normalizeArguments( arguments );
19+
var fCallBack = "";
20+
21+
// If Method & argument Hash Results, switch the results struct
22+
if (structKeyExists( this._mockArgResults, argsHashKey) ) {
23+
// Check if it is a callback
24+
if (isStruct( this._mockArgResults[ argsHashKey ]) &&
25+
structKeyExists( this._mockArgResults[ argsHashKey ], "type" ) &&
26+
structKeyExists( this._mockArgResults[ argsHashKey ], "target" ) ) {
27+
fCallBack = this._mockArgResults[ argsHashKey ].target;
28+
} else {
29+
// switch context and key
30+
results = this._mockArgResults;
31+
resultsKey = argsHashKey;
32+
}
33+
}
34+
35+
// Get the statemachine counter
36+
if (isSimpleValue( fCallBack) ) {
37+
resultsLen = arrayLen( results[ resultsKey ] );
38+
}
39+
40+
// Get the callback counter, if it exists
41+
if (structKeyExists( this._mockCallbacks, resultsKey) ) {
42+
callbackLen = arrayLen( this._mockCallbacks[ resultsKey ] );
43+
}
44+
45+
// Log the Method Call
46+
this._mockMethodCallCounters[ listFirst( resultsKey, "|" ) ] = this._mockMethodCallCounters[ listFirst( resultsKey, "|" ) ] + 1;
47+
48+
// Get the CallCounter Reference
49+
internalCounter = this._mockMethodCallCounters[listFirst(resultsKey,"|")];
50+
arrayAppend( this._mockCallLoggers["sendSSEComment"], arguments );
51+
}
52+
</cfscript>

‎public/testbox/system/stubs/30942F4D0BCB6139072EF27C66218715.cfm‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
<cfscript>
2+
variables[ "checkError" ] = variables[ "tmp_checkError_30942F4D0BCB6139072EF27C66218715" ];
3+
this[ "checkError" ] = variables[ "tmp_checkError_30942F4D0BCB6139072EF27C66218715" ];
4+
5+
// Clean up
6+
structDelete( variables, "tmp_checkError_30942F4D0BCB6139072EF27C66218715" );
7+
structDelete( this, "tmp_checkError_30942F4D0BCB6139072EF27C66218715" );
8+
public any function tmp_checkError_30942F4D0BCB6139072EF27C66218715(
9+
10+
) output=true {
11+
12+
var results = this._mockResults;
13+
var resultsKey = "checkError";
14+
var resultsCounter = 0;
15+
var internalCounter = 0;
16+
var resultsLen = 0;
17+
var callbackLen = 0;
18+
var argsHashKey = resultsKey & "|" & this.mockBox.normalizeArguments( arguments );
19+
var fCallBack = "";
20+
21+
// If Method & argument Hash Results, switch the results struct
22+
if (structKeyExists( this._mockArgResults, argsHashKey) ) {
23+
// Check if it is a callback
24+
if (isStruct( this._mockArgResults[ argsHashKey ]) &&
25+
structKeyExists( this._mockArgResults[ argsHashKey ], "type" ) &&
26+
structKeyExists( this._mockArgResults[ argsHashKey ], "target" ) ) {
27+
fCallBack = this._mockArgResults[ argsHashKey ].target;
28+
} else {
29+
// switch context and key
30+
results = this._mockArgResults;
31+
resultsKey = argsHashKey;
32+
}
33+
}
34+
35+
// Get the statemachine counter
36+
if (isSimpleValue( fCallBack) ) {
37+
resultsLen = arrayLen( results[ resultsKey ] );
38+
}
39+
40+
// Get the callback counter, if it exists
41+
if (structKeyExists( this._mockCallbacks, resultsKey) ) {
42+
callbackLen = arrayLen( this._mockCallbacks[ resultsKey ] );
43+
}
44+
45+
// Log the Method Call
46+
this._mockMethodCallCounters[ listFirst( resultsKey, "|" ) ] = this._mockMethodCallCounters[ listFirst( resultsKey, "|" ) ] + 1;
47+
48+
// Get the CallCounter Reference
49+
internalCounter = this._mockMethodCallCounters[listFirst(resultsKey,"|")];
50+
arrayAppend( this._mockCallLoggers["checkError"], arguments );
51+
52+
if (resultsLen neq 0) {
53+
if (internalCounter gt resultsLen) {
54+
resultsCounter = internalCounter - ( resultsLen * fix( ( internalCounter - 1 ) / resultsLen ) );
55+
return results[ resultsKey ][ resultsCounter ];
56+
} else {
57+
return results[ resultsKey ][ internalCounter ];
58+
}
59+
}
60+
61+
if ( callbackLen neq 0 ) {
62+
fCallBack = this._mockCallbacks[ resultsKey ].first();
63+
return fCallBack( argumentCollection : arguments );
64+
}
65+
66+
if ( not isSimpleValue( fCallBack ) ){
67+
return fCallBack( argumentCollection : arguments );
68+
}
69+
}
70+
</cfscript>

‎public/testbox/system/stubs/754BEF48E30B63BC11E518FA73C07785.cfm‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<cfscript>
2+
variables[ "sendSSEEvent" ] = variables[ "tmp_sendSSEEvent_754BEF48E30B63BC11E518FA73C07785" ];
3+
this[ "sendSSEEvent" ] = variables[ "tmp_sendSSEEvent_754BEF48E30B63BC11E518FA73C07785" ];
4+
5+
// Clean up
6+
structDelete( variables, "tmp_sendSSEEvent_754BEF48E30B63BC11E518FA73C07785" );
7+
structDelete( this, "tmp_sendSSEEvent_754BEF48E30B63BC11E518FA73C07785" );
8+
public void function tmp_sendSSEEvent_754BEF48E30B63BC11E518FA73C07785(
9+
10+
) output=true {
11+
12+
var results = this._mockResults;
13+
var resultsKey = "sendSSEEvent";
14+
var resultsCounter = 0;
15+
var internalCounter = 0;
16+
var resultsLen = 0;
17+
var callbackLen = 0;
18+
var argsHashKey = resultsKey & "|" & this.mockBox.normalizeArguments( arguments );
19+
var fCallBack = "";
20+
21+
// If Method & argument Hash Results, switch the results struct
22+
if (structKeyExists( this._mockArgResults, argsHashKey) ) {
23+
// Check if it is a callback
24+
if (isStruct( this._mockArgResults[ argsHashKey ]) &&
25+
structKeyExists( this._mockArgResults[ argsHashKey ], "type" ) &&
26+
structKeyExists( this._mockArgResults[ argsHashKey ], "target" ) ) {
27+
fCallBack = this._mockArgResults[ argsHashKey ].target;
28+
} else {
29+
// switch context and key
30+
results = this._mockArgResults;
31+
resultsKey = argsHashKey;
32+
}
33+
}
34+
35+
// Get the statemachine counter
36+
if (isSimpleValue( fCallBack) ) {
37+
resultsLen = arrayLen( results[ resultsKey ] );
38+
}
39+
40+
// Get the callback counter, if it exists
41+
if (structKeyExists( this._mockCallbacks, resultsKey) ) {
42+
callbackLen = arrayLen( this._mockCallbacks[ resultsKey ] );
43+
}
44+
45+
// Log the Method Call
46+
this._mockMethodCallCounters[ listFirst( resultsKey, "|" ) ] = this._mockMethodCallCounters[ listFirst( resultsKey, "|" ) ] + 1;
47+
48+
// Get the CallCounter Reference
49+
internalCounter = this._mockMethodCallCounters[listFirst(resultsKey,"|")];
50+
arrayAppend( this._mockCallLoggers["sendSSEEvent"], arguments );
51+
}
52+
</cfscript>

‎public/testbox/system/stubs/B7681C49C5125395612F24D97559A4C2.cfm‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
<cfscript>
2+
variables[ "initSSEStream" ] = variables[ "tmp_initSSEStream_B7681C49C5125395612F24D97559A4C2" ];
3+
this[ "initSSEStream" ] = variables[ "tmp_initSSEStream_B7681C49C5125395612F24D97559A4C2" ];
4+
5+
// Clean up
6+
structDelete( variables, "tmp_initSSEStream_B7681C49C5125395612F24D97559A4C2" );
7+
structDelete( this, "tmp_initSSEStream_B7681C49C5125395612F24D97559A4C2" );
8+
public any function tmp_initSSEStream_B7681C49C5125395612F24D97559A4C2(
9+
10+
) output=true {
11+
12+
var results = this._mockResults;
13+
var resultsKey = "initSSEStream";
14+
var resultsCounter = 0;
15+
var internalCounter = 0;
16+
var resultsLen = 0;
17+
var callbackLen = 0;
18+
var argsHashKey = resultsKey & "|" & this.mockBox.normalizeArguments( arguments );
19+
var fCallBack = "";
20+
21+
// If Method & argument Hash Results, switch the results struct
22+
if (structKeyExists( this._mockArgResults, argsHashKey) ) {
23+
// Check if it is a callback
24+
if (isStruct( this._mockArgResults[ argsHashKey ]) &&
25+
structKeyExists( this._mockArgResults[ argsHashKey ], "type" ) &&
26+
structKeyExists( this._mockArgResults[ argsHashKey ], "target" ) ) {
27+
fCallBack = this._mockArgResults[ argsHashKey ].target;
28+
} else {
29+
// switch context and key
30+
results = this._mockArgResults;
31+
resultsKey = argsHashKey;
32+
}
33+
}
34+
35+
// Get the statemachine counter
36+
if (isSimpleValue( fCallBack) ) {
37+
resultsLen = arrayLen( results[ resultsKey ] );
38+
}
39+
40+
// Get the callback counter, if it exists
41+
if (structKeyExists( this._mockCallbacks, resultsKey) ) {
42+
callbackLen = arrayLen( this._mockCallbacks[ resultsKey ] );
43+
}
44+
45+
// Log the Method Call
46+
this._mockMethodCallCounters[ listFirst( resultsKey, "|" ) ] = this._mockMethodCallCounters[ listFirst( resultsKey, "|" ) ] + 1;
47+
48+
// Get the CallCounter Reference
49+
internalCounter = this._mockMethodCallCounters[listFirst(resultsKey,"|")];
50+
arrayAppend( this._mockCallLoggers["initSSEStream"], arguments );
51+
52+
if (resultsLen neq 0) {
53+
if (internalCounter gt resultsLen) {
54+
resultsCounter = internalCounter - ( resultsLen * fix( ( internalCounter - 1 ) / resultsLen ) );
55+
return results[ resultsKey ][ resultsCounter ];
56+
} else {
57+
return results[ resultsKey ][ internalCounter ];
58+
}
59+
}
60+
61+
if ( callbackLen neq 0 ) {
62+
fCallBack = this._mockCallbacks[ resultsKey ].first();
63+
return fCallBack( argumentCollection : arguments );
64+
}
65+
66+
if ( not isSimpleValue( fCallBack ) ){
67+
return fCallBack( argumentCollection : arguments );
68+
}
69+
}
70+
</cfscript>

‎public/testbox/system/stubs/C383511C024245809F1F73E0CA52E220.cfm‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
<cfscript>
2+
variables[ "poll" ] = variables[ "tmp_poll_C383511C024245809F1F73E0CA52E220" ];
3+
this[ "poll" ] = variables[ "tmp_poll_C383511C024245809F1F73E0CA52E220" ];
4+
5+
// Clean up
6+
structDelete( variables, "tmp_poll_C383511C024245809F1F73E0CA52E220" );
7+
structDelete( this, "tmp_poll_C383511C024245809F1F73E0CA52E220" );
8+
public any function tmp_poll_C383511C024245809F1F73E0CA52E220(
9+
10+
) output=true {
11+
12+
var results = this._mockResults;
13+
var resultsKey = "poll";
14+
var resultsCounter = 0;
15+
var internalCounter = 0;
16+
var resultsLen = 0;
17+
var callbackLen = 0;
18+
var argsHashKey = resultsKey & "|" & this.mockBox.normalizeArguments( arguments );
19+
var fCallBack = "";
20+
21+
// If Method & argument Hash Results, switch the results struct
22+
if (structKeyExists( this._mockArgResults, argsHashKey) ) {
23+
// Check if it is a callback
24+
if (isStruct( this._mockArgResults[ argsHashKey ]) &&
25+
structKeyExists( this._mockArgResults[ argsHashKey ], "type" ) &&
26+
structKeyExists( this._mockArgResults[ argsHashKey ], "target" ) ) {
27+
fCallBack = this._mockArgResults[ argsHashKey ].target;
28+
} else {
29+
// switch context and key
30+
results = this._mockArgResults;
31+
resultsKey = argsHashKey;
32+
}
33+
}
34+
35+
// Get the statemachine counter
36+
if (isSimpleValue( fCallBack) ) {
37+
resultsLen = arrayLen( results[ resultsKey ] );
38+
}
39+
40+
// Get the callback counter, if it exists
41+
if (structKeyExists( this._mockCallbacks, resultsKey) ) {
42+
callbackLen = arrayLen( this._mockCallbacks[ resultsKey ] );
43+
}
44+
45+
// Log the Method Call
46+
this._mockMethodCallCounters[ listFirst( resultsKey, "|" ) ] = this._mockMethodCallCounters[ listFirst( resultsKey, "|" ) ] + 1;
47+
48+
// Get the CallCounter Reference
49+
internalCounter = this._mockMethodCallCounters[listFirst(resultsKey,"|")];
50+
arrayAppend( this._mockCallLoggers["poll"], arguments );
51+
52+
if (resultsLen neq 0) {
53+
if (internalCounter gt resultsLen) {
54+
resultsCounter = internalCounter - ( resultsLen * fix( ( internalCounter - 1 ) / resultsLen ) );
55+
return results[ resultsKey ][ resultsCounter ];
56+
} else {
57+
return results[ resultsKey ][ internalCounter ];
58+
}
59+
}
60+
61+
if ( callbackLen neq 0 ) {
62+
fCallBack = this._mockCallbacks[ resultsKey ].first();
63+
return fCallBack( argumentCollection : arguments );
64+
}
65+
66+
if ( not isSimpleValue( fCallBack ) ){
67+
return fCallBack( argumentCollection : arguments );
68+
}
69+
}
70+
</cfscript>

0 commit comments

Comments
 (0)