fix(crypto): validate ciphertext length before nonce extraction in AES Decrypt - #7227
fix(crypto): validate ciphertext length before nonce extraction in AES Decrypt#7227Siraryansingh wants to merge 2 commits into
Conversation
…S Decrypt Validate that the base64 decoded ciphertext length is at least gcm.NonceSize() in Decrypt before attempting to extract the nonce slice. Returning an error prevents potential runtime panics on truncated or malformed ciphertext inputs. Fixes pipe-cd#7215 Signed-off-by: Aryan Singh <aryansingh.as1012@gmail.com>
✅ Deploy Preview for pipecd-site canceled.
|
|
👋 Hi @Siraryansingh, welcome to PipeCD and thanks for opening your first pull request! We’re really happy to have you here Before your PR gets merged, please check a few important things below. Helpful resources
DCO Sign-offAll commits must include a In case you forget to sign-off your commit(s), follow these steps: For the last commit: git commit --amend --signoff
git push --force-with-leaseFor multiple commits: git rebase --signoff origin/master
git push --force-with-leaseRun checks locallyBefore pushing updates, please run: make checkThis runs the same checks as CI and helps catch issues early. 💬 Need help?If anything is unclear, feel free to ask in this PR or join us on the CNCF Slack in the #pipecd channel. Thanks for contributing to PipeCD! ❤️ |
There was a problem hiding this comment.
Pull request overview
This PR hardens the AES-GCM decryption path in pkg/crypto by validating the decoded ciphertext length before slicing out the nonce, preventing a runtime panic on malformed/truncated inputs (per #7215).
Changes:
- Added a nonce-size length guard in
AESEncryptDecrypter.Decrypt()to return an error instead of panicking on short ciphertext. - Added a regression unit test covering short and empty base64 ciphertext inputs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/crypto/aes.go | Adds decoded-ciphertext length validation before nonce extraction in AES-GCM decrypt. |
| pkg/crypto/aes_test.go | Adds regression coverage for short/empty ciphertext inputs to ensure decrypt returns an error instead of panicking. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| nonceSize := gcm.NonceSize() | ||
| if len(encrypted) < nonceSize { | ||
| return "", fmt.Errorf("ciphertext is shorter than nonce size") | ||
| } |
There was a problem hiding this comment.
Hey @Siraryansingh , I noticed your PR also addresses #7215. I had previously worked on the same AES-GCM short ciphertext panic, and the fixes seem quite similar. Just wanted to mention the overlap so we can avoid duplicating efforts.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Aryan Singh <siraryansingh2005@gmail.com>
|
@Siraryansingh This issue is already fixed on master by #7217, which also addresses #7215. |
What this PR does / why we need it:
Validates that base64-decoded ciphertext is at least
gcm.NonceSize()bytes long before extracting the nonce slice inDecrypt()(pkg/crypto/aes.go).If the decoded ciphertext is shorter than
gcm.NonceSize(),Decrypt()now returns an error gracefully instead of causing an out-of-range slice panic.Which issue(s) this PR fixes:
Fixes #7215
How was this tested?:
TestAESDecryptShortCiphertextunit tests inpkg/crypto/aes_test.gocovering short base64 ciphertext and empty inputgo test -v ./pkg/crypto