Bug Description
SecureData.Decrypt() catches FormatException and CryptographicException and silently returns the raw input as plaintext. This masks key mismatches, corrupted ciphertext, and tampered data — the caller has no way to distinguish between a successful decryption and a silent failure. In a security-sensitive context (service credentials), this could lead to plaintext secrets being passed where encrypted data was expected, or tampered ciphertext being accepted without detection.
Actual Behavior
// SecureData.cs lines 194-204
catch (FormatException)
{
// Graceful fallback for Base64 decoding errors
return payload.ToString();
}
catch (CryptographicException)
{
// Graceful fallback for key mismatches or corrupted ciphertext
return payload.ToString();
}
When decryption fails for any reason, the original (encrypted or corrupted) payload is returned as-is to the caller. The caller assumes it received a valid decrypted value.
Expected Behavior
Cryptographic failures should either:
- Throw an exception so the caller can handle the error explicitly, or
- Return a clear failure indicator (e.g. a result object with a success flag) and log a warning
Silent fallback to plaintext should only be used for the intentional migration path (unencrypted legacy data), not for decryption errors on data that was supposed to be encrypted.
Suggested Fix
Separate the legacy plaintext fallback from actual decryption errors:
catch (FormatException ex)
{
Logger.Warn($"Base64 decoding failed during decryption: {ex.Message}");
throw new InvalidOperationException("Failed to decrypt payload: invalid format.", ex);
}
catch (CryptographicException ex)
{
Logger.Warn($"Decryption failed — possible key mismatch or corrupted data: {ex.Message}");
throw new InvalidOperationException("Failed to decrypt payload: cryptographic error.", ex);
}
If backwards compatibility requires keeping the fallback, at minimum add logging so failures are observable:
catch (FormatException ex)
{
Logger.Warn($"Decryption fallback triggered (FormatException): {ex.Message}");
return payload.ToString();
}
catch (CryptographicException ex)
{
Logger.Warn($"Decryption fallback triggered (CryptographicException): {ex.Message}");
return payload.ToString();
}
Environment
- File:
src/Servy.Core/Security/SecureData.cs
- Lines: 194-204
Bug Description
SecureData.Decrypt()catchesFormatExceptionandCryptographicExceptionand silently returns the raw input as plaintext. This masks key mismatches, corrupted ciphertext, and tampered data — the caller has no way to distinguish between a successful decryption and a silent failure. In a security-sensitive context (service credentials), this could lead to plaintext secrets being passed where encrypted data was expected, or tampered ciphertext being accepted without detection.Actual Behavior
When decryption fails for any reason, the original (encrypted or corrupted) payload is returned as-is to the caller. The caller assumes it received a valid decrypted value.
Expected Behavior
Cryptographic failures should either:
Silent fallback to plaintext should only be used for the intentional migration path (unencrypted legacy data), not for decryption errors on data that was supposed to be encrypted.
Suggested Fix
Separate the legacy plaintext fallback from actual decryption errors:
If backwards compatibility requires keeping the fallback, at minimum add logging so failures are observable:
Environment
src/Servy.Core/Security/SecureData.cs