Skip to content

Commit

Permalink
Unsoundness notice for gix-attributes (kstring integration) (#2027)
Browse files Browse the repository at this point in the history
* Unsoundness notice for gix-attributes (kstring integration)

gix-attributes was found by @ssbr to be unsound, as reported in
GitoxideLabs/gitoxide#1460. This adds an
informational notice for that, as discussed in comments there.

It looks like the affected code, having been introduced in
GitoxideLabs/gitoxide#400, was present in all
versions of the crate prior to the fix in 0.22.3 (which was one of
the bugs fixed in GitoxideLabs/gitoxide#1462).

Co-authored-by: Devin Jeanpierre <jeanpierreda@google.com>

* Small adjustments for advisory

This makes some minor changes to the advisory description to adapt
the text from GitoxideLabs/gitoxide#1460 to be
an advisory. For the most part it has remained the same. Changes:

* Express the claim of unsoundness with more confidence, since it
  has been reviewed by the maintainer.

* Modify the link to the affected code to point to the latest tag
  for gix-attributes that has that code. The original link was to
  a branch, so it was broken when the fix was applied.

* Apply inline code formatting in a few more places, where doing
  so improves stylistic consistency.

---------

Co-authored-by: Devin Jeanpierre <jeanpierreda@google.com>
  • Loading branch information
EliahKagan and ssbr authored Jul 24, 2024
1 parent 0e7413f commit 884aaa1
Showing 1 changed file with 23 additions and 0 deletions.
23 changes: 23 additions & 0 deletions crates/gix-attributes/RUSTSEC-0000-0000.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
```toml
[advisory]
id = "RUSTSEC-0000-0000"
package = "gix-attributes"
date = "2024-07-24"
url = "https://github.com/Byron/gitoxide/issues/1460"
informational = "unsound"

[versions]
patched = [">= 0.22.3"]
```

# The kstring integration in gix-attributes is unsound

`gix-attributes` (in [`state::ValueRef`](https://github.com/Byron/gitoxide/blob/gix-attributes-v0.22.2/gix-attributes/src/state.rs#L19-L27)) unsafely creates a `&str` from a `&[u8]` containing non-UTF8 data, with the justification that so long as nothing reads the `&str` and relies on it being UTF-8 in the `&str`, there is no UB:

```rust
// SAFETY: our API makes accessing that value as `str` impossible, so illformed UTF8 is never exposed as such.
```

The problem is that the non-UTF8 `str` **is** exposed to outside code: first to the `kstring` crate itself, which requires UTF-8 in its documentation and may have UB as a consequence of this, but also to `serde`, where it propagates to e.g. `serde_json`, `serde_yaml`, etc., where the same problems occur.

This is not sound, and it could cause further UB down the line in these places that can view the `&str`.

0 comments on commit 884aaa1

Please # to comment.