71 lines
2.6 KiB
Diff
71 lines
2.6 KiB
Diff
From 0923d82f5c3ac8cf6c99108be2ad9260f2a61f6c Mon Sep 17 00:00:00 2001
|
|
From: CoraleSoft <82213665+Coralesoft@users.noreply.github.com>
|
|
Date: Sun, 26 Oct 2025 17:43:39 +1300
|
|
Subject: [PATCH] Fix timing attack in PKCS1v15 padding validation
|
|
|
|
Replaces variable-time separator search with constant-time implementation to mitigate Marvin Attack (CVE-2022-4304). Uses bitwise operations to avoid data-dependent timing leaks.
|
|
|
|
Fixes three timing vulnerabilities:
|
|
- Variable-time while loop
|
|
- Early return on invalid padding
|
|
- Variable-length memcpy operation
|
|
|
|
Fixes #1247
|
|
---
|
|
pkcspad.cpp | 36 ++++++++++++++++++++++++++++--------
|
|
1 file changed, 28 insertions(+), 8 deletions(-)
|
|
|
|
diff --git a/pkcspad.cpp b/pkcspad.cpp
|
|
index 7f5cd2099..456e52015 100644
|
|
--- a/pkcspad.cpp
|
|
+++ b/pkcspad.cpp
|
|
@@ -100,20 +100,40 @@ DecodingResult PKCS_EncryptionPaddingScheme::Unpad(const byte *pkcsBlock, size_t
|
|
// Require block type 2.
|
|
invalid = (pkcsBlock[0] != 2) || invalid;
|
|
|
|
- // skip past the padding until we find the separator
|
|
- size_t i=1;
|
|
- while (i<pkcsBlockLen && pkcsBlock[i++]) { // null body
|
|
- }
|
|
+ // Constant-time separator search to mitigate timing attacks (Marvin Attack, CVE-2022-4304)
|
|
+ // Scan every byte to find first zero separator without variable-time loop termination
|
|
+ size_t separatorIndex = 0;
|
|
+ size_t foundSeparator = 0;
|
|
+
|
|
+ for (size_t j = 1; j < pkcsBlockLen; j++)
|
|
+ {
|
|
+ // Check if current byte is zero (separator)
|
|
+ size_t isZero = (pkcsBlock[j] == 0) ? 1 : 0;
|
|
+ size_t notFoundYet = 1 - foundSeparator;
|
|
+
|
|
+ // Constant-time conditional: record position using bitwise ops
|
|
+ // Equivalent to: if (isZero && notFoundYet) separatorIndex = j;
|
|
+ size_t mask = -(isZero & notFoundYet); // all 1s if true, all 0s if false
|
|
+ separatorIndex = (separatorIndex & ~mask) | (j & mask);
|
|
+
|
|
+ // Mark that we found a separator
|
|
+ foundSeparator |= isZero;
|
|
+ }
|
|
+
|
|
+ // Position after the separator
|
|
+ size_t i = separatorIndex + 1;
|
|
CRYPTOPP_ASSERT(i==pkcsBlockLen || pkcsBlock[i-1]==0);
|
|
|
|
size_t outputLen = pkcsBlockLen - i;
|
|
invalid = (outputLen > maxOutputLen) || invalid;
|
|
+ invalid = (foundSeparator == 0) || invalid; // No separator found
|
|
|
|
- if (invalid)
|
|
- return DecodingResult();
|
|
-
|
|
+ // Always perform memcpy to avoid timing leak from early return
|
|
+ // This ensures both valid and invalid padding take the same code path
|
|
std::memcpy (output, pkcsBlock+i, outputLen);
|
|
- return DecodingResult(outputLen);
|
|
+
|
|
+ // Return error on invalid padding, otherwise return decoded length
|
|
+ return invalid ? DecodingResult() : DecodingResult(outputLen);
|
|
}
|
|
|
|
// ********************************************************
|