Skip to content

Fix MaxBytesReader limit breaking assignee requests and Decrypt panic on short input#17

Merged
cedricfung merged 4 commits into
mainfrom
copilot/review-v0-3-0-to-v0-4-0
Mar 30, 2026
Merged

Fix MaxBytesReader limit breaking assignee requests and Decrypt panic on short input#17
cedricfung merged 4 commits into
mainfrom
copilot/review-v0-3-0-to-v0-4-0

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 30, 2026

Security review of v0.3.0→v0.4.0 changes found two issues:

MaxBytesReader limit too small (API breaking change)

The 1024-byte MaxBytesReader limit silently breaks the assignee feature. Measured request sizes:

Request type Size
SIGN (basic) ~1005 bytes
SIGN + assignee ~1535 bytes
SIGN + assignee + rotate ~1637 bytes

Basic SIGN barely fits; anything with an assignee is rejected outright. Increased to 4096.

crypto.Decrypt panics on short ciphertext

b[:aead.NonceSize()] panics with index-out-of-range when b is shorter than 12 bytes (GCM nonce). Reachable via API with a short base64-encoded data field. Added bounds check returning nil, consistent with existing error-handling pattern in the function.

// Before: panics if len(b) < 12
func Decrypt(secret, b []byte) []byte {
    aes, _ := aes.NewCipher(secret)
    aead, _ := cipher.NewGCM(aes)
    nonce := b[:aead.NonceSize()]  // panic

// After: returns nil on invalid input
    if len(b) < aead.NonceSize()+aead.Overhead() {
        return nil
    }

📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.

Copilot AI and others added 3 commits March 30, 2026 09:22
Security review findings from v0.3.0 to v0.4.0:

1. MaxBytesReader limit (1024 bytes) was too restrictive - broke the
   assignee feature (requests with assignee are ~1535 bytes) and was
   dangerously close to the limit for basic SIGN requests (~1005 bytes).
   Increased to 4096 bytes.

2. crypto.Decrypt panicked on short input (< 12 bytes) due to missing
   length validation before slice operations. Added proper bounds
   checking that returns nil on short/invalid input (consistent with
   existing error-handling pattern).

Agent-Logs-Url: https://github.com/MixinNetwork/tip/sessions/11386e20-35e9-4396-ae29-bdcc1e2b30b3

Co-authored-by: cedricfung <2269238+cedricfung@users.noreply.github.com>
Copilot AI changed the title [WIP] Review changes for v0.3.0 to v0.4.0 regarding security and API issues Fix MaxBytesReader limit breaking assignee requests and Decrypt panic on short input Mar 30, 2026
Copilot AI requested a review from cedricfung March 30, 2026 09:30
@cedricfung cedricfung marked this pull request as ready for review March 30, 2026 10:05
@cedricfung cedricfung merged commit 516642c into main Mar 30, 2026
@cedricfung cedricfung deleted the copilot/review-v0-3-0-to-v0-4-0 branch March 30, 2026 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants