11 ms·
Breaking Git with a carriage return and cloning RCE
- deleted 1y ago[deleted]
- therealmarv 1y agoguess I have to wait a bit more... no update to git 2.50.1 on Homebrew yet.
- leipert 1y agohttps://github.com/Homebrew/homebrew-core/pull/229423 https://github.com/Homebrew/homebrew-core/pull/229423 or brew install git --HEAD
- Fishkins 1y agoThanks for making that PR! A regular `brew install git` installs 2.50.1 for me now.
- therealmarv 1y agoThanks for the PR. I was also looking into it briefly and did not understand where the SHA256 for the various architectures are coming from and so I gave up before creating the PR (now I understand it's created automatically by a bot).
- dwrodri 1y ago[flagged]
- asplake 1y agoThe article refutes that somewhat: > I find this particularly interesting because this isn't fundamentally a problem of the software being written in C. These are logic errors that are possible in nearly all languages, the common factor being this is a vulnerability in the interprocess communication of the components (either between git and external processes, or within the components of git itself).
- bpt3 1y agoAs mentioned in the article, this is a logic error that has nothing to do with C strings.
- eptcyka 1y agoWhilst true, there’s a swathe of modern tooling that will aide in marshalling data for IPC. Would you not agree that if protobuf, json or yaml were used, it’d be far less likely for this bug have slipped in?
- alexvitkov 1y agoIn isolation, for any one particular bug, yes, but if you start applying this logic to everything, even problems as simple as reading some bytes from a file, you end up with a heao of dependencies for the most mundane things. We've tried that, it's bad.
- gpm 1y agoOn the contrary, we've tried it and it works great.
- sunshowers 1y agoNo, I think in general you should trust other people with experience in an area more than your own naive self. Division of labor and all that. There are exceptions, as always, but using dependencies is good as a first approximation.
- eptcyka 1y agoI don't believe we must apply any guideline ad absurdum. Using a battle tested marshalling/serialization library is clearly the way to go most often. Of course, one can still construct difficult to parse XML and JSON or any other blob for any given format, but the chances that bad input will result in an RCE are lower.
- bangaladore 1y agoThe OC was about language choice. You can use protobuf, json or yaml in C as well. In general, though, all these can be wildly overkill for many tasks. At some point you just need to write good code and actually test it.
- jerf 1y agoAs the article says: "I find this particularly interesting because this isn't fundamentally a problem of the software being written in C. These are logic errors that are possible in nearly all languages, the common factor being this is a vulnerability in the interprocess communication of the components (either between git and external processes, or within the components of git itself). It is possible to draw a parallel with CRLF injection as seen in HTTP (or even SMTP smuggling)." You can write this in any language. None of them will stop you. I'm on the cutting edge of "stop using C", but this isn't C's fault.
- gpm 1y agoYou can, but in languages like python/java/go/rust/... you wouldn't, because you wouldn't write serialization/de-serialization code by hand but call out to a battle hardened library. This vulnerability is the fault of the C ecosystem where there is no reasonable project level package manager so everyone writes everything from scratch. It's exacerbated by the combination of a lack of generics (rust/java's solution), introspection (java/python's solution), and poor preprocessor in C (go's solution) so it wouldn't even be easy to make a ergonomic general purpose parser.
- bangaladore 1y agoI have a feeling that this code was developed before any of those languages were widely popular and before their package managers or packages were mature. This file was written like 20 years ago.
- gpm 1y agoSure, I'm not trying to assign blame to Linus for deciding to write git in C, I'm saying that modern tooling (not C) would prevent the bug with reasonably high probability and that that's a factor when deciding what to do going forwards.
- shakna 1y agoPython's pathlib wouldn't help you here, it can encode the necessary bits. Especially with configparser - it's 20 year old configuration reader. Java's story is worse. What part of this would be prevented by another language? You'd need to switch your data format to something like json, toml, etc. to prevent this from the outset. But JSON was first standardised 25 years ago, and AJAX wasn't invented when this was written. JSON was a fledgling and not widely used yet. I guess we had netrc - but that's not standardised and everyone implements it differently. Same story for INI. There was XML - at a time when it was full of RCEs, and everyone was acknowledging that its parser would be 90% of your program. Would you have joined the people disparaging json at the time as reinventing xml? This vulnerability is the fault of data formats not being common enough to be widely invented yet.
- mrkeen 1y agoC programmers don't see C problems. They see logic errors that are possible in any language.
- dietr1ch 1y agoRunning with scissors isn't a problem. The problem is stabbing yourself with them. Isn't it obvious?
- alexvitkov 1y agoWe keep getting RCEs in C because tons of useful programs are written in C. If someone writes a useful program in Rust, we'll get RCEs in Rust.
- dietr1ch 1y agoIt's not that only C programs are useful. It's that subtle mistakes on C result in more catastrophic vulnerabilities. Make a mistake in application code in a language like, say Java, and you'll end up with an exception.
- deleted 1y ago[deleted]
- School-Cotton 1y agoThere are a lot of useful programs written in Rust nowadays. This comment might have made more sense like 5 years ago.
- alexvitkov 1y agoI mean Photoshop, Excel, Figma, etc -- programs I can show someone and say "Look, here's a cool thing you couldn't do with a computer before, but now you can!" Nothing I've seen in rust cuts meets that bar for me.
- School-Cotton 1y agomaterialize.com (disclosure: I worked there for five years) is entirely written in Rust and as far as I know the first system to support incremental view maintenance over the full range of SQL semantics (including e.g. fully precise non-windowed joins, recursive queries, etc.) with a SQL interface (Postgres dialect).
- smileson2 1y agoIf only it were just the c code that was causing people to be owned lol
- deleted 1y ago[deleted]
- tuetuopay 1y agoUsing other languages would likely fix the issue but as a side-effect. Most people would expect a C-vs-Rust comparison so I’ll take Go as an example. Nobody would write the configuration parsing code by hand, and just use whatever TOML library available at hand for Go. No INI shenanigans, and people would just use any available stricter format (strings must be quoted in TOML). So yeah, Rust and Go and Python and Java and Node and Ruby and whatnot would not have the bug just by virtue of having a package manager. The actual language is irrelevant. However, whatever the language, the same hand implementation would have had the exact same bug.
- deanc 1y agoWould homebrew itself be problematic here? Does it do recursive cloning? At least a cursory glance at the repo suggests it might: https://github.com/Homebrew/brew/blob/700d67a85e0129ab8a893ff69246943479e33df1/Library/Homebrew/download_strategy.rb#L1173 https://github.com/Homebrew/brew/blob/700d67a85e0129ab8a893f...
- msgodel 1y agoIt would be odd if it didn't. Although the goal of homebrew is to execute the code in the repo. The only situation where the RCE here is a problem is if you clone github repos containing data you don't want to execute. That's fairly unusual.
- leni536 1y agoThe question is whether recursive submodule checkout happens after some integrity/signature validation or before. The RCE can be an issue in the latter case.
- johncolanduoni 1y agoThere would also have to be a compromise of the transport (i.e. a MITM of HTTPS or SSH) to use this in most practical scenarios.
- leni536 1y agoIt still weakens the security, otherwise why bother with integrity/signature checks if you trust the git remote?
- deleted 1y ago[deleted]
- armchairhacker 1y ago> The result of all this, is when a submodule clone is performed, it might read one location from path = ..., but write out a different path that doesn’t end with the ^M. How does this achieve “remote code execution” as the article states? How serious is it from a security perspective? > I'm not sharing a PoC yet, but it is an almost trivial modification of an exploit for CVE-2024-32002. There is also a test in the commit fixing it that should give large hints. EDIT: from the CVE-2024-32002 > Repositories with submodules can be crafted in a way that exploits a bug in Git whereby it can be fooled into writing files not into the submodule's worktree but into a .git/ directory. This allows writing a hook that will be executed while the clone operation is still running, giving the user no opportunity to inspect the code that is being executed. So a repository can contain a malicious git hook. Normally git hooks aren’t installed by ‘git clone’, but this exploit allows one to, and a git hook can run during the clone operation.
- pinoy420 1y ago[flagged]
- jmb99 1y agoIn this case, there is more than enough information given to make an exploit trivial for anyone actually investigating the issue. I don’t see a reason to distribute PoC exploit code here, it’s fairly clear what the problem is as well as at least one possible RCE implementation.
- zahlman 1y agoA well-written article should still have spelled out what armchairhacker did. The article spent many paragraphs working through what seemed to me like a very obvious chain of reasoning that didn't need so much hand-holding, and then left me completely bewildered at the end with the last step. And even with the explanation, I'm still not sure why writing the files to a different path allows the hook to be written. (Surely Git knows that it's still in the middle of a recursive clone operation and that it therefore shouldn't accept any hooks?)
- 10000truths 1y agoThis is a big problem with using ad-hoc DSLs for config - there's often no formal specification for the grammar, and so the source of truth for parsing is spread between the home-grown serialization implementation and the home-grown deserialization implementation. If they get out of sync (e.g. someone adds new grammar to the parser but forgets to update the writer), you end up with a parser differential, and tick goes the time bomb. The lesson: have one source of truth, and generate everything that relies on it from that.
- ajross 1y agoNitpick: the DSL here ("ini file format") is arguably ad-hoc, but it's extremely common and well-understood, and simple enough to make a common law specification work well enough in practice. The bug here wasn't due to the format. What you're actually complaining about is the hand-coded parser[1] sitting in the middle of a C program like a bomb waiting to go off. And, yes, that nonsense should have died decades ago. There are places for clever hand code, even in C, even in the modern world. Data interchange is very not much not one of them. Just don't do this. If you want .ini, use toml. Use JSON if you don't. Even YAML is OK. Those with a penchant for pain like XML. And if you have convinced yourself your format must be binary (you're wrong, it doesn't), protobufs are there for you. But absolutely, positively, never write a parser unless your job title is "programming language author". Use a library for this, even if you don't use libraries for anything else. [1] Fine fine, lexer. We are nitpicking, after all.
- heisenbit 1y agoHow many hand crafted lexers dealing with lf vs. cr-lf encodings do exist? My guess is n > ( number of people who coded > 10 KSLOC ).
- hnlmorg 1y agoI’ve written a fair few lexers in my time. My general approach for CR is to simply ignore the character entirely. If CR is used correctly in windows, then its behaviour is already covered by the LF case (as required for POSIX systems) and if CR is used incorrectly then you end up with all kinds of weird edge cases. So you’re much better off just jumping over that character entirely.
- smaudet 1y agoIs it just me or is the font an eyestrain on this blog?
- MisterTea 1y agoI am not sure if there's bias on my part after reading your comment but yes, it is bothersome.
- metalliqaz 1y agoyeah I see what you mean. it's like the anti-aliasing is broken
- deleted 1y ago[deleted]
- b0a04gl 1y agowhy tf is git still running submodule hooks during clone at all. like think. youre cloning a repo which you didnt write it or audit it. and git just... runs a post checkout hook from a submodule it just fetched off the internet. even with this CRLF bug fixed, thats still bananas
- HappMacDonald 1y agoI completely disagree with author's (oft quoted here in comments) statement: > I find this particularly interesting because this isn't fundamentally a problem of the software being written in C. These are logic errors that are possible in nearly all languages For Christ's sake, Turing taught us that any error in one language is possible in any other language. You can even get double free in Rust if you take the time to build an entire machine emulator and then run something that uses Malloc in the ensuing VM. Rust and similar memory safe languages can emulate literally any problem C can make a mine field out of.. but logic errors being "possible" to perform are significantly different from logic errors being the first tool available to pull out of one's toolbox. Other comments have cited that in non-C languages a person would be more likely to reach for a security-hardened library first, which I agree might be helpful.. but replies to those comments also correctly point out that this trades one problem for another with dependency hell, and I would add on top of that the issue that a widely relied upon library can also increase the surface area of attack when a novel exploit gets found in it. Libraries can be a very powerful tool but neither are they a panacea. I would argue that the real value in a more data-safe language (be that Rust or Haskell or LISP et al) is in offering the built-in abstractions which lend themselves to more carefully modeling data than as a firehose of octets which a person then assumes they need to state-switch over like some kind of raw Turing machine. "Parse, don't validate" is a lot easier to stick to when you're coding in a language designed with a precept like that in mind vs a language designed to be only slightly more abstract than machine code where one can merely be grateful that they aren't forced to use jump instructions for every control flow action.
- lilyball 1y agoI can easily see this bug happening in Rust. At some level you need to transform your data model into text to write out, and to parse incoming text. If you want to parse linewise you might use BufRead::lines(), and then write a parser for those lines. That parser won't touch CRs at all, which means when you do the opposite and write the code that serializes your data model back to lines, it's easy to forget that you need to avoid having a trailing CR, since CR appears nowhere else in your code.
- 1y ago
- Lockal 1y ago"trivial modification of an existing exploit"... Why git does not use Landlock? I know it is Linux-only, but why? "git clone" should only have r/o access to config directory and r/w to clone directory. And no subprocesses. In every exploit demo: "Yep, <s>it goes to a square hole</s> it launches a calculator".
- TheDong 1y ago> no subprocesses I guess you're okay with breaking all git hooks, including post-checkout, because those are subprocesses as a feature. You can always run your git operations in a container with seccomp or such if you're not using any of the many features that it breaks
- Spivak 1y agoThis would also break custom commands. Which if you don't know about it, is a pretty cool feature. Drop a git-something executable in your path and you can execute it as git something.
- byearthithatius 1y agoWhy is this helpful? Just add the executable itself to path and execute it with "something" instead of "git something". Why are we making git an intermediary ? I am kind of stupid and this is genuine.
- wbl 1y agoBecause something might make less sense on its own or conflict with another tool.
- mkesper 1y agoBecause it's thematically a part of a git workflow.
- joseda-hg 1y agoBecause if it's part of the repo, you don't depend on the host to take the extra step, which, if you're working from ephemeral instances or places where that step would have to be repeated, is a god send
- sugarpimpdorsey 1y ago[flagged]
- metalliqaz 1y agoevery other day of the year the rest of the industry is laughing in git
- deleted 1y ago[deleted]
- TacticalCoder 1y ago[flagged]
- zahlman 1y agoSuppose the system call to list a directory examined the place on the disk where a filename should be, and found bytes representing ASCII control characters. Should it deny the existence of the corresponding file? Assume disk corruption? Something else? After all, maybe (this is admittedly more theoretical than practical) those bytes map to something else in the current locale. It's not like modern Windows which assumes the filenames are all UTF-16.
- 0x457 1y agoBecause filenames (and all other strings) are just bags of bytes on unix based systems.
- TacticalCoder 1y ago[dead]
- JdeBP 1y agoReading someone quote Jon Postel in the context of CR+LF brings back memories. * https://jdebp.uk/FGA/qmail-myths-dispelled.html#MythAboutBareLFs https://jdebp.uk/FGA/qmail-myths-dispelled.html#MythAboutBar... "that may not be the most sensible advice now", says M. Leadbeater today. We were saying that a lot more unequivocally, back in 2003. (-: As Mark Crispin said then, the interpretations that people put on it are not what M. Postel would have agreed with. Back in the late 1990s, Daniel J. Bernstein did the famous analysis that noted that parsing and quoting when converting between human-readable and machine-readable is a source of problems. And here we are, over a quarter of a century later, with a quoter that doesn't quote CRs (and even after the fix does not look for all whitespace characters). Amusingly, git blame says that the offending code was written 19 years ago, around the time that Daniel J. Bernstein was doing the 10 year retrospective on the dicta about parsing and quoting. * https://github.com/git/git/commit/cdd4fb15cf06ec1de588bee4576509857d8e2cb4 https://github.com/git/git/commit/cdd4fb15cf06ec1de588bee457... * https://cr.yp.to/qmail/qmailsec-20071101.pdf https://cr.yp.to/qmail/qmailsec-20071101.pdf I suppose that we just have to keep repeating the lessons that were already hard learned in the 20th century, and still apply in the 21st.
- lossolo 1y agoIt seems like Homebrew still provides a vulnerable version, the same goes for Debian Bookworm.
- tomku 1y ago2.50.1 is available on Homebrew now, for anyone seeing this.
- dwheeler 1y agoAh yes, yet ANOTHER vulnerability caused because Linux and most Unixes allow control characters in filenames. This ability's primary purpose appears to be to enable attacks and to make it significantly more difficult to write correct code. For example, you're not supposed to exchange filenames a line at a time, since filenames can contain newlines. See my discussion here: https://dwheeler.com/essays/fixing-unix-linux-filenames.html https://dwheeler.com/essays/fixing-unix-linux-filenames.html One piece of good news: POSIX recently added xargs -0 and find -print0, making it a little easier to portably handle such filenames. Still, it's a pain. I plan to complete my "safename" Linux module I started years ago. When enabled, it prevents creating filenames in certain cases such as those with control characters. It won't prevent all problems, but it's a decent hardening mechanism that prevents problems in many cases.
- layer8 1y agoYou can get similar vulnerabilities with Unicode normalization, with mismatched code pages/character encodings, or, as the article points out, with a case-insensitive file system. That's not to say that control characters should be allowed in file names, but there's an inherent risk whenever byte sequences are being decoded or normalized into something else.
- dwheeler 1y agoNot to the same degree, though, and the arguments for status quo are especially weak. There are reasonable arguments pro and con case-insensitive filenames. Character encoding issues are dwindling, since most systems just use utf-8 for filename encoding (as there is no mechanism for indicating the encoding of each specific filename), and using utf-8 consistently in filenames supports filenames in arbitrary languages. Control characters in filenames have no obviously valuable use case, they appear to be allowed only because "it's always been allowed". That is not a strong argument for them. Some systems do not allow them, with no obvious ill effects.
- Cloudef 1y agoI think better idea is to make git use user namespaces and sandbox itself to the clone directory so it literally cannot write/read outside of it. This prevents path traversal attacks and limits the amount of damage RCE could do. Filenames really aren't the problem.
- IshKebab 1y agoZero surprise there's a bug in git's quoting. That code is mental.
- IshKebab 1y agoDownvotes from people who haven't actually read the git quoting code. I have.
- _k2vp 1y agoReproduced the issue after a bit: https://github.com/acheong08/CVE-2025-48384 https://github.com/acheong08/CVE-2025-48384 Then immediately went to update my git version. Still not up on Arch yet. Will refrain from pulling anything but I bet it'll take quite a while for most people to upgrade. Putting it in any reasonable popular repo where there are perhaps automated pulls will be interesting.
- orblivion 1y agoSo this was disclosed before patching? With all of the alarming "here's how we can pwn your machine" posts turning out to be months after the fact, I figured by now that these blog posts all happen after all the distros have long patched it. It seems like it would be appropriate to make it clear "this is important now" vs "don't worry you probably already patched this" in the headline to save our time for those who aren't just reading these posts out of interest.
- _lvbh 1y agoCommits fixing the bug date back around 3 or 4 weeks. The patched release came 3 weeks ago. Perhaps some parties weren't informed that it's security critical (Homebrew, Arch, etc) and are now scrambling
- SchemaLoad 1y agoJust went and checked and the latest version on macOS is over a year old.. >git version 2.39.5 (Apple Git-154)
- orblivion 1y agoAm I reading this wrong? As of this writing it all says "vulnerable". https://security-tracker.debian.org/tracker/CVE-2025-48384 https://security-tracker.debian.org/tracker/CVE-2025-48384
- dgl 1y agoI'm not privy to the exact communications that happened, but per the Ubuntu changelog they prepared a patch a week ago[1] (which is about the normal timeline for notification per[2]). Homebrew is not on the distros list, so likely wouldn't have got an early notification. Arch is, but remember "The Arch Security Team is a group of volunteers"[3]. [1]: https://launchpad.net/ubuntu/+source/git/1:2.43.0-1ubuntu7.3 https://launchpad.net/ubuntu/+source/git/1:2.43.0-1ubuntu7.3 [2]: https://oss-security.openwall.org/wiki/mailing-lists/distros https://oss-security.openwall.org/wiki/mailing-lists/distros [3]: https://wiki.archlinux.org/title/Arch_Security_Team https://wiki.archlinux.org/title/Arch_Security_Team
- mrheosuper 1y agoit's 2025 and we still can't decide to use /r, /n or /r/n.
- account42 1y agoOf course we can, it's just that some people decide wrong.
- layer8 1y agoLone CR died with classic Mac OS over 20 years ago, I think we can ignore that. Lone LF is arguably a Unix-ism, everything else is/was using CRLF. Except that Unix text files are becoming ubiquitous, while existing protocols and formats, as well as Windows conventions, are stuck with CRLF. There’s really no good way out.
- pjc50 1y agoI'm on team "Windows should just accept and change to write and read CR and '/'. beginning the decades long transition process for those". Most of the APIs accept '/', and most of the software accepts CR-only. I think even Microsoft have noticed this, which is why WSL exists to provide an island of Linux-flavored open source tooling inside a Windows environment.
- layer8 1y agoI think you mean LF, not CR. The problem with changing the behavior with regard to CRLF is exactly that it would introduce vulnerabilities like the present one here, because some software would still apply the old behavior while others apply the new one. Stuff like https://portswigger.net/web-security/request-smuggling/advanced#request-smuggling-via-crlf-injection https://portswigger.net/web-security/request-smuggling/advan.... Directory separators are another can of worms. A lot of functionality in Windows is driven by command-line invocations taking slash-prefixed options, where it’s crucial that they are syntactically distinct from file system paths. I don’t think a transition is possible without an unacceptable amount of compatibility breakage.
- 1y ago
- jftuga 1y agoI wrote a CLI program to determine and detect the end-of-line format, tabs, bom, and nul characters. It can be installed via Homebrew or you can download standalone binaries for all platforms: https://github.com/jftuga/chars https://github.com/jftuga/chars
- capitol_ 1y agoI'm confused about the timeline here, the tag for 2.50.1 is from 2025-06-16 ( https://github.com/git/git/tags https://github.com/git/git/tags ). And the date for 2.50.1 on https://git-scm.com https://git-scm.com is also 2025-06-16. But it seems like almost no distributions have patched it yet https://security-tracker.debian.org/tracker/CVE-2025-48386 https://security-tracker.debian.org/tracker/CVE-2025-48386 (debian as an example) And the security advisory is from yesterday: https://github.com/git/git/security/advisories/GHSA-4v56-3xvj-xvfr https://github.com/git/git/security/advisories/GHSA-4v56-3xv... Did git backdate the release?
- CodesInChaos 1y agoThat's the time the commit was authored, not the time the tag was published.
- tempodox 1y agoIt's often the most inconspicuous stuff that leads to highly undesirable consequences.
- unit149 1y ago[dead]