Skip to content

Conversation

@terrier989
Copy link
Member

We need to check that the list is modifiable before overwriting.

Copilot AI review requested due to automatic review settings November 21, 2025 21:33
Copilot finished reviewing on behalf of terrier989 November 21, 2025 21:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves the safety of overwriting sensitive byte data by replacing try-catch blocks with proactive runtime type checks. Instead of attempting to overwrite and catching exceptions, the code now uses identical(bytes.runtimeType, Uint8List) to verify the list is modifiable before calling fillRange.

Key Changes:

  • Replaced exception-based approach with runtime type checking using identical(runtimeType, Uint8List)
  • Simplified code by removing try-catch blocks in favor of conditional checks
  • Applied pattern consistently across SensitiveBytes.destroy() and cipher overwriting operations

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
cryptography/lib/src/cryptography/sensitive_bytes.dart Replaced try-catch with runtime type check in destroy() method to verify bytes are modifiable before overwriting
cryptography/lib/src/cryptography/cipher.dart Added runtime type checks before overwriting bytes in decryptString() and encryptString() methods to prevent errors on unmodifiable lists

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@terrier989 terrier989 force-pushed the fix/overwriting branch 2 times, most recently from e39f7ad to c8a67ba Compare November 21, 2025 22:51
@terrier989 terrier989 merged commit 67f2d3e into master Nov 21, 2025
24 checks passed
@terrier989 terrier989 deleted the fix/overwriting branch November 21, 2025 23:12
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