LWN: Comments on "A security audit of Git"

A security audit of Git

smammy — Wed, 01 Feb 2023 13:11:03 +0000

This is great. Now let's see a usability audit of Git.

A security audit of Git

moltonel — Sun, 29 Jan 2023 21:01:07 +0000

Yes, the Rust used in that article is purposefully close to the original C, but that makes it very unidiomatic.
I didn't read the C git code very closely, so it's possible that it does something complex that also wouldn't be straightforward in Rust, but from a quick look, idiomatic Rust would work well here and be safe.

Rust often nudges you towards using compatible integer types instead of casting, or towards explicit arithmetic, like `saturating_sub()`. You'd be using iterators instead of a manual end-of-string check. Less wheels to reinvent in each project: that C `strbuf_split_buf()` looks suspiciously like Rust's various `split()` methods, perhaps with a `.owned()` in the chain if you really needs owned strings instead of references (often not needed in Rust, due to str not needing a null terminator). Things like `malloc()`ing a buffer and `memcpy()`ing too much data into it just doesn't happen in normal Rust: you just grab a `Vec<u8>`, with enough preallocated space if you have that info, and then `.extend()` it with your source data as a slice.

A security audit of Git

ms-tg — Sun, 29 Jan 2023 00:23:05 +0000

Would they be amenable to writing differently in Rust though? The article was showing direct translation of the C as written into Rust, not what would seem to be idiomatic Rust?

A security audit of Git

mathstuf — Thu, 26 Jan 2023 17:30:31 +0000

While I agree, the Rust code would have gotten a lot more "that looks weird" feelings (from me at least) because (at least) the integer casts are noisy. Implicit casts in C and C++ meanwhile tend to skate through review.

A security audit of Git

wtarreau — Thu, 26 Jan 2023 13:21:36 +0000

I agree and that's what I noticed there too, most of them were "well, in fact they would still be a problem there".

A security audit of Git

MrWim — Wed, 25 Jan 2023 18:12:17 +0000

Well, seeing as the bandwagon is rolling, I'll jump on too. From the summary:

> Git is a distributed version control system that allows developers to collaborate on software development. It is integrated into popular packaging systems, including Golang modules, Rust cargo, and NodeJS NPM.

I believe cargo uses libgit2 (via the git2 crate) rather than the git command line[^1]. So I don't think this report helps harden cargo.

Relatedly: the rust foundation are currently sponsoring @Byron[2] to replace the use of libgit2 with gitoxide[3] in cargo[4]. gitoxide is a reimplementation of git in rust. It's my favourite RIIR[^5] project. It's being implemented with great focus on correctness and speed - with some very impressive benchmarks[6]. @Byron publishes a progress report on github every month, which is great for following along with the project. Here's a 2022 retrospective[7].

[^1]: Unless you specify `git-fetch-with-cli = true`
[2]: https://github.com/Byron/
[3]: https://github.com/Byron/gitoxide/
[4]: https://github.com/rust-lang/cargo/pull/11448
[^5]: Rewrite it in Rust
[6]: https://github.com/Byron/gitoxide/discussions/579
[6]: https://github.com/Byron/gitoxide/discussions/681

A security audit of Git

moltonel — Wed, 25 Jan 2023 17:53:05 +0000

Yes, sorry about the meme, I often find those plugs annoying too. But I found this one worth sharing : it gives a clear concise explanation of the vulnerabilities found (which is useful in its own right), and it's very pragmatic about what Rust does and doesn't help with. I feel it's more targeted at existing Rust users than potential converts : more "here are real-world vulnerabilities examples to keep in mind even when writing Rust" than "lol, 4 CVEs that Rust would have avoided".

A security audit of Git

wtarreau — Wed, 25 Jan 2023 15:46:31 +0000

I was wondering how many minutes it would take to see a rust plug after a security audit report :-)

A security audit of Git

moltonel — Wed, 25 Jan 2023 10:40:41 +0000

https://litchipi.github.io/infosec/2023/01/24/git-code-au... is also worth a read.