sdk: add bounds check when instantiating Keypair from byte array
#34817
Conversation
|
Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis. |
| @@ -44,6 +44,11 @@ impl Keypair { | |||
|
|
|||
| /// Recovers a `Keypair` from a byte array | |||
| pub fn from_bytes(bytes: &[u8]) -> Result<Self, ed25519_dalek::SignatureError> { | |||
| if bytes.len() < ed25519_dalek::KEYPAIR_LENGTH { | |||
There was a problem hiding this comment.
I know it doesn't matter for slice indexing, but is there any reason this should be catching longer byte slices as well? (ie !=)
There was a problem hiding this comment.
i considered it, but decided to punt under the impression that someone is likely abusing the poorly defined api. clearly this method signature should be
fn from_bytes(bytes: &[u8; 64]) -> Result<Self, NotTheDamnDependencysError>but here we are
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #34817 +/- ##
=========================================
- Coverage 81.7% 81.7% -0.1%
=========================================
Files 825 825
Lines 223249 223250 +1
=========================================
- Hits 182609 182598 -11
- Misses 40640 40652 +12 |
Problem
no bounds check on candidate by array in
Keypair::from_bytes()h/t: certik
Summary of Changes
add one