Cryptographically verifiable, distributed dependency reviews
Add the last reviewed version to Cargo.toml / [dependencies]:
dtb = "0.1.3"
Please, use mobile in landscape.
Filter reviews clicking on the numbers in the summary.
Full column names in tooltip hints: rating Negative, rating Neutral, rating Positive, rating Strong, thoroughness, understanding, reviews count.
Liberal use of unsafe
and sparse validation of in puts indices and offsets.
In principle, the dtb format lends itself well to this use as the file format
itself already requires the alignment of many members and takes care to have
naturally packed structs–with aligned members but no padding.
It is thus possibly safe to map many parts of an immutable input directly to
structs marked as repr(C)
, which also correctly appears.
However, the unsafe
blocks contain only few indications of consideration of
their safety. Sometimes alignment checks appear obviously above but most
iterators implicitely trust their callers on the alignment of internal
buffers. It also seems that not all functions relying on unsafe
preconditions are marked unsafe
. This applies to internal functions only
but may make the crate more brittle than necessary.
Another antipattern is that of a byte output buffer: A StructItem
offers
reading its value as strings or a u32
list. But instead of an iterator over
the backing memory the implementation takes an mutable reference to a byte
slice, manually aligns it to fit the output type, casts it, and writes the
data types &'_ str
and u32
. I have not found concrete misbehaviour from
this but it seems awkward.
© bestia.dev 2021, MIT Licence, Version: 2021.1208.1729
Open source repository for this web app: https://github.com/bestia-dev/cargo_crev_web/
Has fuzzing integration and been scrutinized by a previous review. I've done
version 0.1.2 in some detail and the only changes merged since then are bug
fix PRs addressing those. Mostly worried about inadvertently introduced bugs
in future changes due to awkward interfaces and implicit trust between
different components within the code.
For example, code in
struct_item.rs
relies on the reader providing only 4byte aligned buffers instead of reasserting that fact as an internal
invariant. However this invariant is relied upon for an aligned pointer load
later.
Similarly some other C style patterns are prominent. Instead of functions
testing an invariant and constructing a result based off of it, boolean
return and construction by the caller is used. Example:
This is fine in its current usage as far as I can tell but it's not very
stable with regards to possible future changes.
This also means that manual arithmetic is used for bounds checks which is
prone to missed overflow considerations etc.
A remaining antipattern is that of a byte output buffer:
StructItem
offersreading its value as strings or a
u32
list. But instead of an iterator overthe backing memory the implementation takes an mutable reference to a byte
slice, manually aligns it to fit the output type, casts it, and writes the
data types
&'_ str
andu32
. I have not found concrete misbehaviour fromthis but it seems awkward.