Conversation
… Key createUserSpecificEncryptionKey() mixed the fully attacker-controlled "name" parameter with the secret serverEncryptionKey via plain byte-wise addition mod 256 - a linear, invertible operation. Since the doGet() endpoint echoes the plaintext baseKey literal alongside its ciphertext for any attacker-chosen name (except the hidden "This Challenge" entry), this is a chosen-key setup that lets an attacker recover serverEncryptionKey byte-by-byte, then decrypt the hidden challenge's own answer. Compounding this, encrypt()/decrypt() used AES/CBC with a fixed all-zero IV, making ciphertexts for the same plaintext+key always identical. Added dedicated createUserSpecificEncryptionKey/encryptUserSpecific/ decryptUserSpecific methods for the user-specific-key path: key derivation now uses SHA-256(serverEncryptionKey || userNameKey) (one-way, so chosen-input outputs reveal nothing about the secret), and encryption uses AES-256-GCM with a random IV per call (IV prepended to ciphertext), matching the pattern already merged for BrokenCrypto3.java. The unrelated, unused generic encrypt()/decrypt()/ decryptUserName() methods (not part of the reported key-derivation bug, and not called anywhere in the codebase) are left untouched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
AES-GCM uses a random IV per encryption, so re-encrypting the same plaintext never reproduces the same ciphertext string. The doPost handler was still comparing the submitted ciphertext against a freshly re-encrypted "expected" ciphertext for string equality - this would have rejected every submission, including correct ones, after the prior commit's switch away from deterministic fixed-IV CBC. Now decrypts the submitted ciphertext with the user's derived key and compares the resulting plaintext against the expected baseKey+salt value instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
🏆 Security Shepherd — CTF Patch Score0 / 40 challenges patched
Commit: No points yet — this commit didn't solve any challenges, so there's nothing on the leaderboard for it. Patch a vulnerability and push again! 💪 |
…lied name The previous commit switched key derivation to SHA-256 to stop an attacker recovering the server's secret key via the old linear byte-addition mixing, but left the "name" value itself coming straight from the request parameter. That's an IDOR via the key-derivation input rather than the usual object-id parameter: submitting another user's username as "name" returns that user's personalised encrypted answers in the response, with no relation to who is actually authenticated. Derive it from the session's own userName instead, so a caller can only ever request their own encrypted answers.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
createUserSpecificEncryptionKey()mixed the fully attacker-controlled "name" parameter with the secretserverEncryptionKeyvia plain byte-wise addition mod 256 - a linear, invertible operation. SincedoGet()echoes the plaintext baseKey literal alongside its ciphertext for any attacker-chosen name, this is a chosen-key setup that lets an attacker recoverserverEncryptionKeybyte-by-byte, then decrypt the hidden challenge's own answer. Compounding this, encryption used AES/CBC with a fixed all-zero IV.Fix
Added dedicated key-derivation/encryption/decryption methods for the user-specific-key path: key derivation now uses SHA-256(serverEncryptionKey || userNameKey) (one-way), and encryption uses AES-256-GCM with a random IV per call, matching the pattern already merged for
BrokenCrypto3.java. Also fixed the submission-comparison logic to decrypt-and-compare plaintext instead of comparing ciphertext strings (required since GCM's random IV means re-encrypting never reproduces the same ciphertext).Test plan
mvn compile,mvn spotless:checkclean, full test suite passesdoGetwith a chosen name produces valid varying-length AES-GCM ciphertexts; a garbage/wrong submission is handled gracefully as "incorrect" rather than erroring; a standalone round-trip test (encrypt viagenerateUserSolutionKeyOnly, decrypt viadecryptUserSpecificSolution) confirms the full pipeline is internally consistent.🤖 Generated with Claude Code