ralexstokes / ssz-rs

Implementation of ethereum's `ssz`
Apache License 2.0
103 stars 41 forks source link

skip some duplicate checks when deserializing fixed types #84

Closed ralexstokes closed 1 year ago

ralexstokes commented 1 year ago

from Oak Security audit:

When deserializing Array, List, and Vector objects, depending on whether they are fixed or variable, one of two functions is called within deserialize_homogeneous_composite - deserialize_variable_homogeneous_composite or deserialize_fixed_homogeneous_composite. The latter is called when the type is fixed, and the corresponding deserialize functions have already validated the length of the object to be a multiple of the default value of that type obtained using T::size_hint. An example for the Array type is the validation performed in ssz-rs/src/array.rs:51-65. Nevertheless, the deserialize_fixed_homogeneous_composite function in lines 71-78 validates whether the modulo of the length of the deserialized object and the default size for its type is different from zero. Bearing in mind the fact that in the previous step this size was multiplied by N, it means that consequently there is no possibility that the modulo will be different from zero. Ultimately, it is therefore a redundant piece of code.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.08 :tada:

Comparison is base (b872969) 75.26% compared to head (42b7852) 75.34%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #84 +/- ## ========================================== + Coverage 75.26% 75.34% +0.08% ========================================== Files 18 18 Lines 857 860 +3 ========================================== + Hits 645 648 +3 Misses 212 212 ``` | [Impacted Files](https://app.codecov.io/gh/ralexstokes/ssz-rs/pull/84?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Alex+Stokes) | Coverage Δ | | |---|---|---| | [ssz-rs/src/de.rs](https://app.codecov.io/gh/ralexstokes/ssz-rs/pull/84?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Alex+Stokes#diff-c3N6LXJzL3NyYy9kZS5ycw==) | `78.18% <100.00%> (-1.82%)` | :arrow_down: | | [ssz-rs/src/list.rs](https://app.codecov.io/gh/ralexstokes/ssz-rs/pull/84?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Alex+Stokes#diff-c3N6LXJzL3NyYy9saXN0LnJz) | `77.38% <100.00%> (+2.38%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.