Common Cryptographic Pitfalls

Introduction

Cryptography is unforgiving. A single mistake—using weak randomness, accepting non-canonical signatures, or mishandling keys—can compromise the entire system. This chapter catalogs the most common cryptographic pitfalls, explains why they're dangerous, and shows you how to avoid them.

Pitfall 1: Weak Random Number Generation

The Mistake

// ❌ WRONG - Predictable randomness
void generateWeakKey() {
    std::srand(std::time(nullptr));  // Seed with current time

    std::uint8_t secretKey[32];
    for (auto& byte : secretKey) {
        byte = std::rand() % 256;  // NOT cryptographically secure
    }

    return SecretKey{Slice{secretKey, 32}};
}

Why It's Dangerous

Predictability:

std::srand(time(NULL)) seeds with seconds since epoch
Attacker knows approximate time of key generation
Can try all possible times (seconds in a day = 86,400)
Tests each seed value → predicts all random numbers
Recovers secret key!

Real-world example:

// Key generated at approximately 2025-10-15 14:30:00 UTC
// Attacker knows this within ±1 hour = 3,600 seconds
// Tries all 3,600 possible seeds
// Generates keys for each
// Checks which key matches public key
// Finds correct seed and regenerates secret key
// Total time: seconds

The Fix

// ✅ CORRECT - Cryptographically secure
SecretKey generateStrongKey() {
    std::uint8_t buf[32];
    beast::rngfill(buf, sizeof(buf), crypto_prng());
    SecretKey sk{Slice{buf, sizeof(buf)}};
    secure_erase(buf, sizeof(buf));
    return sk;
}

Detection

// Red flags in code review:
std::srand(...)        // ❌
std::rand()            // ❌
std::mt19937           // ❌ (not cryptographic)
std::uniform_*         // ❌ (if used with non-crypto RNG)

// Good signs:
crypto_prng()          // ✅
RAND_bytes()           // ✅
randomSecretKey()      // ✅

Pitfall 2: Memory Leakage

The Mistake

// ❌ WRONG - Secret key remains in memory
void processTransaction() {
    std::string secretKeyHex = loadFromConfig();
    auto secretKey = parseHex(secretKeyHex);

    auto signature = sign(pk, sk, tx);

    // Function returns
    // secretKeyHex still in memory!
    // secretKey still in memory!
    // Can be recovered from memory dump
}

Why It's Dangerous

Attack vectors:

  1. Core dumps: Process crashes, core dump contains secrets

  2. Swap files: Memory paged to disk, secrets written to swap

  3. Hibernation: All memory written to hibernation file

  4. Memory inspection: Debugger or malware reads process memory

  5. Cold boot: RAM retains data briefly after power off

The Fix

// ✅ CORRECT - Explicit cleanup
void processTransaction() {
    std::string secretKeyHex = loadFromConfig();
    auto secretKey = parseSecretKey(secretKeyHex);

    auto signature = sign(pk, secretKey, tx);

    // Explicit cleanup
    secure_erase(
        const_cast<char*>(secretKeyHex.data()),
        secretKeyHex.size());

    // secretKey is SecretKey object - RAII erases automatically
}

// ✅ BETTER - Use RAII throughout
void processTransaction() {
    SecretKey sk = loadSecretKey();  // RAII-protected

    auto signature = sign(pk, sk, tx);

    // sk destructor automatically erases
}

Detection

// Code review checklist:
□ Are temporary buffers with secrets erased?
□ Are std::string secrets explicitly cleaned?
□ Are secrets in RAII wrappers (SecretKey)?
□ Is secure_erase called before function returns?
□ Are there early returns that skip cleanup?

Pitfall 3: Accepting Non-Canonical Signatures

The Mistake

// ❌ WRONG - Doesn't enforce canonicality
bool verifyTransaction(Transaction const& tx) {
    return verify(
        tx.publicKey,
        tx.data,
        tx.signature,
        false);  // mustBeFullyCanonical = false ❌
}

Why It's Dangerous

Signature malleability allows attacks:

// Alice creates transaction
Transaction tx = Payment{ /* ... */ };
Signature sig1 = sign(alice.sk, tx);
uint256 txID1 = hash(tx, sig1);

// Attacker creates malleated signature
Signature sig2 = malleate(sig1);  // Still valid!
uint256 txID2 = hash(tx, sig2);   // Different ID!

// Both signatures valid, but different transaction IDs
// Applications tracking txID1 won't see confirmation (appears as txID2)
// Can cause confusion, double-spend attempts, invalid dependent transactions

The Fix

// ✅ CORRECT - Enforce canonical signatures
bool verifyTransaction(Transaction const& tx) {
    return verify(
        tx.publicKey,
        tx.data,
        tx.signature,
        true);  // mustBeFullyCanonical = true ✅
}

Detection

// Search for:
verify(..., false)          // ❌ Likely wrong
verifyDigest(..., false)    // ❌ Likely wrong

// Should be:
verify(..., true)           // ✅ Correct
verifyDigest(..., true)     // ✅ Correct

Pitfall 4: Key Reuse Across Contexts

The Mistake

// ❌ WRONG - Same key for everything
SecretKey masterKey = loadKey();

// Use for transactions
auto txSig = sign(masterPK, masterKey, transaction);

// Use for peer handshakes
auto handshakeSig = sign(masterPK, masterKey, sessionData);

// Use for validator messages
auto validationSig = sign(masterPK, masterKey, ledgerHash);

Why It's Dangerous

Cross-protocol attacks:

// Signature from one context used in another
// Example:
// 1. Attacker captures handshake signature
// 2. Replays it as transaction signature
// 3. If data happens to match, signature validates!
// 4. Unauthorized transaction executed

Key compromise amplification:

// If one key compromised:
// - ALL contexts compromised
// - Transactions, handshakes, validations
// - Total system failure

The Fix

// ✅ CORRECT - Separate keys for separate purposes
struct NodeKeys {
    SecretKey accountKey;      // For account transactions only
    SecretKey nodeKey;          // For peer communication only
    SecretKey validationKey;    // For validator messages only
};

// Use appropriate key for each context
auto txSig = sign(keys.accountPK, keys.accountKey, transaction);
auto handshakeSig = sign(keys.nodePK, keys.nodeKey, sessionData);
auto validationSig = sign(keys.validationPK, keys.validationKey, ledgerHash);

Additional Protection: Hash Prefixes

// Even with same key, use different prefixes
auto txHash = sha512Half(HashPrefix::transaction, txData);
auto handshakeHash = sha512Half(HashPrefix::manifest, handshakeData);

// Different prefixes → different hashes → cross-context attack prevented

Pitfall 5: Timing Attacks on Comparisons

The Mistake

// ❌ WRONG - Variable-time comparison
bool compareSignatures(Slice const& a, Slice const& b) {
    if (a.size() != b.size())
        return false;

    for (size_t i = 0; i < a.size(); ++i) {
        if (a[i] != b[i])
            return false;  // Early exit leaks position of first mismatch!
    }

    return true;
}

Why It's Dangerous

Timing side-channel:

// Attacker measures time for comparison to fail
// If first byte wrong: returns quickly
// If first byte correct, second wrong: takes longer
// Byte-by-byte, extract the secret signature

// Example:
Signature guess = "00000000...";
Time: 1 microsecond      // First byte wrong

guess = "A0000000...";
Time: 1.1 microseconds   // First byte right, second wrong

guess = "AB000000...";
Time: 1.2 microseconds   // First two bytes right

// Repeat until full signature recovered

The Fix

// ✅ CORRECT - Constant-time comparison
bool compareSignatures(Slice const& a, Slice const& b) {
    if (a.size() != b.size())
        return false;

    // Use constant-time comparison
    return CRYPTO_memcmp(a.data(), b.data(), a.size()) == 0;
}

// Or use OpenSSL's implementation
bool compareSignatures(Slice const& a, Slice const& b) {
    if (a.size() != b.size())
        return false;

    return OPENSSL_memcmp(a.data(), b.data(), a.size()) == 0;
}

Detection

// Red flags:
if (signature[i] != expected[i])  // ❌ Byte-by-byte comparison
if (signature == expected)         // ❌ May use std::memcmp
memcmp(sig, expected, size)       // ❌ Not constant-time

// Good signs:
CRYPTO_memcmp(...)                // ✅
OPENSSL_memcmp(...)               // ✅
Constant-time comparison library  // ✅

Pitfall 6: Insufficient Key Length

The Mistake

// ❌ WRONG - Only 64 bits of key material
std::uint8_t weakKey[8];  // 8 bytes = 64 bits
crypto_prng()(weakKey, sizeof(weakKey));

Why It's Dangerous

Security level = bits / 2 (for symmetric keys)
64-bit key = 2^32 operations to break
With modern hardware: breakable in seconds/minutes

Modern GPUs can try billions of keys per second
2^32 = 4,294,967,296
At 1 billion/second = 4.3 seconds

The Fix

// ✅ CORRECT - Full 256 bits
std::uint8_t strongKey[32];  // 32 bytes = 256 bits
crypto_prng()(strongKey, sizeof(strongKey));

// Security level: 2^128 operations
// Even quantum computers won't break this (2^128 > 10^38)

Guideline

Minimum security levels:
- 128 bits: Adequate (2^64 operations)
- 256 bits: Strong (2^128 operations, quantum-resistant)
- 512 bits: Overkill (but doesn't hurt)

XRPL uses 256 bits for secret keys (standard)

Pitfall 7: Rolling Your Own Crypto

The Mistake

// ❌ WRONG - Custom encryption
std::vector<uint8_t> myEncrypt(
    std::vector<uint8_t> const& data,
    std::vector<uint8_t> const& key)
{
    std::vector<uint8_t> encrypted;
    for (size_t i = 0; i < data.size(); ++i) {
        encrypted.push_back(data[i] ^ key[i % key.size()]);
    }
    return encrypted;  // XOR "encryption" - trivially broken!
}

Why It's Dangerous

Expertise required:

  • Cryptography is subtle and unforgiving

  • Experts spend years studying attack techniques

  • Small mistakes lead to complete breaks

  • Standard algorithms battle-tested by thousands of experts

Common mistakes in custom crypto:

  1. Weak key scheduling

  2. Poor mode of operation

  3. No authentication

  4. Padding oracle vulnerabilities

  5. Timing side-channels

  6. Implementation bugs

The Fix

// ✅ CORRECT - Use standard, vetted libraries
#include <openssl/evp.h>

// Use AES-256-GCM (authenticated encryption)
std::vector<uint8_t> encrypt(
    std::vector<uint8_t> const& plaintext,
    std::vector<uint8_t> const& key,
    std::vector<uint8_t> const& iv)
{
    // Use OpenSSL's EVP interface
    // Handles all the complexity correctly
    // ...
}

Rule

NEVER implement your own:

  • Encryption algorithms

  • Hash functions

  • Signature schemes

  • Random number generators

  • Key derivation functions

ALWAYS use:

  • OpenSSL

  • libsodium

  • Other well-vetted libraries

  • Standard algorithms (AES, SHA, etc.)

Pitfall 8: Ignoring Error Returns

The Mistake

// ❌ WRONG - Doesn't check return value
void generateKey() {
    uint8_t key[32];
    RAND_bytes(key, 32);  // What if this fails?

    // Continue using potentially-uninitialized key!
    useKey(key);
}

Why It's Dangerous

// If RAND_bytes fails:
// - key[] contains uninitialized data
// - Might be zeros
// - Might be predictable
// - Might be previous key material!

// Using failed key:
// - Weak encryption
// - Predictable signatures
// - Complete compromise

The Fix

// ✅ CORRECT - Check and handle errors
SecretKey generateKey() {
    uint8_t buf[32];

    if (RAND_bytes(buf, 32) != 1) {
        // RNG failed - this is critical!
        Throw<std::runtime_error>("RNG failure - cannot continue");
    }

    SecretKey sk{Slice{buf, 32}};
    secure_erase(buf, 32);
    return sk;
}

Guideline

// ALWAYS check return values for:
RAND_bytes()              // Random generation
secp256k1_*()            // secp256k1 operations
ed25519_*()              // Ed25519 operations
EVP_*()                  // OpenSSL EVP operations
SSL_*()                  // SSL/TLS operations

// Fail loudly on error:
// - Throw exception
// - Return error code
// - Log and abort
// NEVER continue with failed crypto operations

Pitfall 9: Hardcoded Secrets

The Mistake

// ❌ WRONG - Secret in source code
const char* API_KEY = "sk_live_51Hx9y2JKLs...";
const char* MASTER_SEED = "sn3nxiW7v8KXzPzAqzyHXbSSKNuN9";

void authenticate() {
    // Use hardcoded secret
}

Why It's Dangerous

Exposure:

  • Source code in version control (git history)

  • Binary contains strings (recoverable)

  • Code reviews expose secrets

  • Logs may print secrets

  • Hard to rotate if compromised

The Fix

// ✅ CORRECT - Load from secure storage
SecretKey loadKey() {
    // Option 1: Environment variable
    auto const seedStr = std::getenv("XRPL_SEED");

    // Option 2: Config file with restricted permissions
    auto const seedStr = readSecureConfig("seed");

    // Option 3: Hardware security module (HSM)
    auto const seedStr = loadFromHSM();

    // Option 4: OS keychain/credential manager
    auto const seedStr = loadFromKeychain();

    // Parse and use
    auto seed = parseSeed(seedStr);
    return generateSecretKey(KeyType::ed25519, seed);
}

Best Practices

1. Never commit secrets to version control
2. Use environment variables or config files
3. Restrict file permissions (600 or 400)
4. Use secrets management systems (Vault, etc.)
5. Rotate secrets regularly
6. Audit access to secrets

Pitfall 10: Insufficient Validation

The Mistake

// ❌ WRONG - Doesn't validate inputs
void processTransaction(std::string const& addressStr) {
    // Assume it's valid
    AccountID account = decodeAddress(addressStr);

    // What if decoding failed?
    // What if address is malformed?
    // Undefined behavior!
}

Why It's Dangerous

Consequences:

  • Using invalid keys → signature verification fails

  • Using malformed data → undefined behavior

  • Skipping validation → security bypasses

  • Accepting bad input → system corruption

The Fix

// ✅ CORRECT - Validate everything
void processTransaction(std::string const& addressStr) {
    // Validate address
    if (!isValidAddress(addressStr)) {
        throw std::invalid_argument("Invalid address format");
    }

    // Decode (will succeed because validated)
    AccountID account = decodeAddress(addressStr);

    // Validate account exists
    if (!ledger.hasAccount(account)) {
        throw std::runtime_error("Account not found");
    }

    // Continue...
}

Validation Checklist

□ Public keys: Correct format? Right size? Valid curve point?
□ Signatures: Canonical? Correct size? Valid encoding?
□ Addresses: Valid checksum? Correct type prefix?
□ Seeds: Valid format? Sufficient entropy?
□ Hashes: Correct size? Expected format?
□ Amounts: Non-negative? Within limits?
□ All user input: Validated before use?

Summary

Common cryptographic pitfalls and how to avoid them:

  1. Weak RNG: Use crypto_prng(), never std::rand()

  2. Memory leaks: Use RAII, call secure_erase()

  3. Non-canonical sigs: Always enforce canonicality

  4. Key reuse: Separate keys for separate purposes

  5. Timing attacks: Use constant-time comparisons

  6. Short keys: Use 256 bits minimum

  7. Custom crypto: Never - use standard libraries

  8. Ignored errors: Always check return values

  9. Hardcoded secrets: Load from secure storage

  10. Missing validation: Validate all inputs

Golden rules:

  • Trust no input

  • Check all returns

  • Erase all secrets

  • Use standard crypto

  • Fail loudly on errors

Last updated