Skip to content

Commit 884aaa1

Browse files
EliahKaganssbr
andauthored
Unsoundness notice for gix-attributes (kstring integration) (#2027)
* 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 <[email protected]> * 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 <[email protected]>
1 parent 0e7413f commit 884aaa1

File tree

1 file changed

+23
-0
lines changed

1 file changed

+23
-0
lines changed
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
```toml
2+
[advisory]
3+
id = "RUSTSEC-0000-0000"
4+
package = "gix-attributes"
5+
date = "2024-07-24"
6+
url = "https://github.com/Byron/gitoxide/issues/1460"
7+
informational = "unsound"
8+
9+
[versions]
10+
patched = [">= 0.22.3"]
11+
```
12+
13+
# The kstring integration in gix-attributes is unsound
14+
15+
`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:
16+
17+
```rust
18+
// SAFETY: our API makes accessing that value as `str` impossible, so illformed UTF8 is never exposed as such.
19+
```
20+
21+
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.
22+
23+
This is not sound, and it could cause further UB down the line in these places that can view the `&str`.

0 commit comments

Comments
 (0)