fix(security): AES key/IV validation, SecureRandom.base64Url, GCM docs + tests#18
Conversation
…s + tests - H1: Validate AES key length (16/24/32 bytes) in constructor - H2: Validate IV length (12 bytes for GCM, 16 bytes for others) in encrypt() and decrypt() - H5: Add base64Url getter to SecureRandom for URL-safe encoding - Add GCM to README supported modes list with ECB deprecation note - Add 5 GCM test cases: round-trip, associated data, wrong AD fails, key sizes (128/192/256), IV uniqueness ref #391, ref #403 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
PR SummaryMedium Risk Overview Extends Written by Cursor Bugbot for commit 6cc0a92. Configure here. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
|
|
||
| String get base64 => convert.base64.encode(_bytes); | ||
|
|
||
| String get base64Url => convert.base64Url.encode(_bytes); |
There was a problem hiding this comment.
base64Url includes padding characters
Medium Severity
SecureRandom.base64Url uses convert.base64Url.encode, which emits padded Base64URL (often ending in =). For “URL-safe random token generation”, padding can still break usage in URL/path/query contexts unless callers remember to strip/encode it, so the getter’s behavior doesn’t match the implied guarantee.
|
|
||
| String get base64 => convert.base64.encode(_bytes); | ||
|
|
||
| String get base64Url => convert.base64Url.encode(_bytes); |
There was a problem hiding this comment.
New base64Url getter appears unused
Low Severity
The new SecureRandom.base64Url getter is not referenced anywhere in the repo (the only base64Url usage in tests refers to dart:convert’s top-level base64Url). This looks like dead API surface that increases maintenance cost without coverage proving it works as intended.
| if (iv != null) { | ||
| if (mode == AESMode.gcm && iv.bytes.length != 12) { | ||
| throw ArgumentError('GCM mode requires a 12-byte IV (96 bits). Got ${iv.bytes.length} bytes.'); | ||
| } else if (mode != AESMode.ecb && mode != AESMode.gcm && iv.bytes.length != 16) { | ||
| throw ArgumentError('AES ${_modes[mode]} mode requires a 16-byte IV (128 bits). Got ${iv.bytes.length} bytes.'); | ||
| } | ||
| } |
There was a problem hiding this comment.
Bug: The new validation in aes.dart requires a 12-byte IV for GCM mode, but the example code in example/aes_gcm.dart uses a 16-byte IV, causing a runtime ArgumentError.
Severity: HIGH
Suggested Fix
Update the example code in example/aes_gcm.dart to use a 12-byte IV, for instance, by changing IV.fromSecureRandom(16) to IV.fromSecureRandom(12). This will align the example with the new validation rules. Alternatively, if supporting variable-length IVs for GCM is desired, the validation logic should be relaxed or removed.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: lib/src/algorithms/aes.dart#L33-L39
Potential issue: A new validation was added in `lib/src/algorithms/aes.dart` that
enforces a strict 12-byte IV length for AES in GCM mode. However, the library's own
example file, `example/aes_gcm.dart`, uses `IV.fromSecureRandom(16)` to generate a
16-byte IV. This mismatch will cause the example code to fail with an `ArgumentError` at
runtime, making it unusable and creating a breaking change for users who might have been
using other IV lengths with GCM mode.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Pull request overview
This pull request enhances AES encryption security by adding key and IV length validation, introduces a URL-safe random token generator, and improves GCM mode documentation and test coverage.
Changes:
- Added AES key length validation enforcing 16/24/32-byte keys (128/192/256-bit)
- Added AES IV length validation: 12 bytes for GCM mode, 16 bytes for other modes
- Added
SecureRandom.base64Urlgetter for URL-safe random token generation - Enhanced GCM documentation in README with AEAD explanation and ECB deprecation note
- Added 5 comprehensive GCM test cases covering round-trip encryption, AAD, key sizes, and IV uniqueness
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lib/src/algorithms/aes.dart | Adds key length validation in constructor and IV length validation in encrypt/decrypt methods |
| lib/src/secure_random.dart | Adds base64Url getter for URL-safe encoding |
| test/encrypt_test.dart | Adds comprehensive GCM test suite with 5 test cases |
| README.md | Updates supported modes list with GCM AEAD description and ECB deprecation note |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (iv != null) { | ||
| if (mode == AESMode.gcm && iv.bytes.length != 12) { | ||
| throw ArgumentError('GCM mode requires a 12-byte IV (96 bits). Got ${iv.bytes.length} bytes.'); | ||
| } else if (mode != AESMode.ecb && mode != AESMode.gcm && iv.bytes.length != 16) { | ||
| throw ArgumentError('AES ${_modes[mode]} mode requires a 16-byte IV (128 bits). Got ${iv.bytes.length} bytes.'); | ||
| } | ||
| } |
There was a problem hiding this comment.
The IV validation logic is duplicated between the encrypt and decrypt methods. Consider extracting this validation into a private helper method to reduce code duplication and ensure consistency. For example, a method like _validateIV(IV? iv) could be called at the start of both encrypt and decrypt methods.
| expect(e1.base64, isNot(equals(e2.base64))); | ||
| }); | ||
| }); | ||
|
|
There was a problem hiding this comment.
The new key and IV validation logic lacks test coverage. While the GCM functional tests are comprehensive, there are no tests verifying that invalid key lengths (e.g., 15, 17, 20 bytes) or invalid IV lengths (e.g., 11, 13, 16 bytes for GCM; 15, 17 bytes for other modes) correctly throw ArgumentError exceptions. The codebase has established patterns for testing validation errors (see test/encrypt_test.dart lines 271-282 for RSA StateError tests). Consider adding a dedicated test group for AES validation errors to ensure the validation logic works correctly and provide clear documentation of expected error behavior.
| group('AES validation errors', () { | |
| test('throws ArgumentError for invalid GCM key lengths', () { | |
| for (final keyLen in [15, 17, 20]) { | |
| expect( | |
| () => Encrypter(AES(Key.fromLength(keyLen), mode: AESMode.gcm)), | |
| throwsA(isA<ArgumentError>()), | |
| ); | |
| } | |
| }); | |
| test('throws ArgumentError for invalid GCM IV lengths', () { | |
| final encrypter = Encrypter(AES(key, mode: AESMode.gcm)); | |
| for (final ivLen in [11, 13, 16]) { | |
| final iv = IV.fromLength(ivLen); | |
| expect( | |
| () => encrypter.encrypt(text, iv: iv), | |
| throwsA(isA<ArgumentError>()), | |
| ); | |
| } | |
| }); | |
| test('throws ArgumentError for invalid IV lengths in non-GCM mode', () { | |
| final encrypter = Encrypter(AES(key, mode: AESMode.cbc)); | |
| for (final ivLen in [15, 17]) { | |
| final iv = IV.fromLength(ivLen); | |
| expect( | |
| () => encrypter.encrypt(text, iv: iv), | |
| throwsA(isA<ArgumentError>()), | |
| ); | |
| } | |
| }); | |
| }); |
## Changelog ## [7.0.0] - 2026-02-24 ### Breaking Changes - **BREAKING**: PBKDF2 iteration count increased from 100 to 600,000 by default (#16) (fixes #16) - Migration: Default `Key.stretch` behavior → explicitly set `iterationCount: 100` for legacy derivation - **BREAKING**: Strict validation for AES key and IV lengths has been added (#18) (fixes #18) - Migration: Unvalidated AES keys/IVs → keys must be 16, 24, or 32 bytes; IVs 12 bytes (GCM) or 16 bytes (others) ### Added - Added `base64Url` getter to `SecureRandom` for URL-safe encoding (#18) (fixes #18) - Added auto-format job to CI workflow and updated config (#16) (fixes #16) ### Changed - Raised `Key.stretch` (PBKDF2) default iterations from 100 to 600,000 per OWASP recommendations (#16) (fixes #16) - Updated `runtime_ci_tooling` dependency to `^0.11.0` ### Deprecated - Deprecated insecure AES ECB mode that leaks plaintext patterns -- will be removed in v8.0.0 (#16) (fixes #16) ### Fixed - Validated AES key lengths (16/24/32 bytes) and IV lengths (12 bytes for GCM, 16 for others) to prevent misuse (#18) (fixes #18) ### Security - Implemented constant-time XOR-accumulation comparison for Fernet HMAC verification to fix timing side-channel attacks (#16) (fixes #16) - Used constant-time XOR comparison for equal-length RSA signature verification to fix timing side-channel attacks (#16) (fixes #16) ## Files Modified ``` .../audit/v7.0.0/explore/breaking_changes.json | 23 +++++ .../audit/v7.0.0/explore/commit_analysis.json | 58 +++++++++++++ .runtime_ci/audit/v7.0.0/explore/pr_data.json | 20 +++++ .runtime_ci/audit/v7.0.0/meta.json | 82 ++++++++++++++++++ .../v7.0.0/version_analysis/version_bump.json | 1 + .../version_analysis/version_bump_rationale.md | 21 +++++ .runtime_ci/autodoc.json | 8 +- .../release_notes/v7.0.0/changelog_entry.md | 25 ++++++ .runtime_ci/release_notes/v7.0.0/contributors.json | 5 ++ .runtime_ci/release_notes/v7.0.0/highlights.md | 4 + .../release_notes/v7.0.0/linked_issues.json | 32 +++++++ .../release_notes/v7.0.0/migration_guide.md | 67 +++++++++++++++ .runtime_ci/release_notes/v7.0.0/release_notes.md | 99 ++++++++++++++++++++++ .../release_notes/v7.0.0/release_notes_body.md | 99 ++++++++++++++++++++++ .runtime_ci/version_bumps/v7.0.0.md | 21 +++++ CHANGELOG.md | 27 ++++++ README.md | 2 + pubspec.yaml | 2 +- 18 files changed, 591 insertions(+), 5 deletions(-) ``` ## Version Bump Rationale # Version Bump Rationale **Decision**: major ## Key Changes - **Security**: Added constant-time XOR comparison in Fernet HMAC verification and RSA signature verification to prevent timing side-channel attacks. - **Security**: Raised the default PBKDF2 iteration count in `Key.stretch` from 100 to 600,000 to align with OWASP recommendations. - **Security**: Added strict validation for AES key lengths (16, 24, or 32 bytes) and IV lengths (12 bytes for GCM, 16 bytes for others). - **Deprecation**: Deprecated `AESMode.ecb` as it is insecure and leaks plaintext patterns. - **CI**: Updated CI workflows to run auto-formatting and handle bot commits more cleanly. ## Breaking Changes - The default `iterationCount` for `Key.stretch` has been changed from `100` to `600000`. This is a breaking change because calling `Key.stretch` without specifying the iteration count will now produce a different derived key. Users relying on the old default to decrypt existing data must update their code to exp... ## Contributors - @tsavo-at-pieces --- Automated release by CI/CD pipeline (Gemini CLI + GitHub Actions) Commits since v6.0.2: 4 Generated: 2026-02-24T01:09:15.979476Z


Summary
SecureRandom.base64Urlgetter for URL-safe random token generationTest plan
dart testfrom package directory)Closes #17
Closes #18
🤖 Generated with Claude Code