Skip to content

Instantly share code, notes, and snippets.

@EliahKagan
Created July 3, 2024 23:28
Show Gist options
  • Save EliahKagan/4c38854781685b5b0d7faccb58e835d0 to your computer and use it in GitHub Desktop.
Save EliahKagan/4c38854781685b5b0d7faccb58e835d0 to your computer and use it in GitHub Desktop.
RUSTSEC gitoxide discussion thread draft comment

This is a draft, not in its final form, of a comment in Byron/gitoxide#1437. At minimum the forthcoming PRs need to be opened and their PR numbers filled in.

Thanks! I've opened # and #. I am not sure if they're ready to go, however, mainly relating to the question of which and how many crates need notices. I believe it is more than one in each case, but significantly fewer than the number of crates listed as affected in the repository-local and GitHub Advisory Database global advisories. I've detailed that and other potential areas of improvement in the PR descriptions.

@Byron You may want to take a look at those PRs, in case you notice any mistakes or have a preference as to how things should be done. I'm particularly interested in a double-check on whether it's okay that I have omitted gix-worktree-state as requiring a notice. This is the same as in other crates that are affected mainly because they consume affected crates, but in the case of gix-worktree-state the behavior really is sort of conceptually centered there, so my belief that it doesn't need a notice deserves some scrutiny. In addition, its code did need to be changed, but I think it may be that dependency resolution would prevent an old unfixed version of it from being used with the new version of gix-worktree.

In addition, and mostly separate from the forthcoming RUSTSEC advisories and the advisory-db PRs, while working on this I noticed that it looks like my listing of gix-fs as affected by the Windows device names vulnerability CVE-2024-35197 (https://github.com/Byron/gitoxide/security/advisories/GHSA-49jc-r788-3fc9) seems to have been mistaken. The key security-related change to gix-fs in #1374, if I understand correctly, was:

https://github.com/Byron/gitoxide/blob/e2e8cf9206058365be50ea899cc4dcc22e923bef/gix-fs/src/stack.rs#L103-L111

While this is an extremely important part of the fix for the directory traversal vulnerability CVE-2024-35186 (https://github.com/Byron/gitoxide/security/advisories/GHSA-7w47-3wg8-547c), it is separate from the case of path components that clash with reserved legacy DOS-style Windows device names like CON and COM1. I think both vulnerabilities' fixes included important improvements in gix-index and gix-worktree, but that the changes in gix-fs were only part of the fix for the directory traversal vulnerability (and of course the changes in gix-ref were only a part of the fix for the reserved names vulnerability).

Assuming that analysis is correct, I was wrong to have listed gix-fs for CVE-2024-35197 (https://github.com/Byron/gitoxide/security/advisories/GHSA-49jc-r788-3fc9) and should remove it from the list of affected crates in the repo-local advisory https://github.com/Byron/gitoxide/security/advisories/GHSA-49jc-r788-3fc9 and submit a PR to do the same in the CVE-2024-35197 global advisory.

Mainly in case there are other changes that should also be made to those advisories, I'm holding off on making those changes for a short while. But you can also let me know if you think I was right about gix-fs originally and mistaken now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment