11 ms·
How I Made a Heap Overflow in Curl
- junon 3y ago[flagged]
- knallfrosch 3y ago> Everyone not happy about this are of course welcome to roll up their sleeves and get working.
- junon 3y agoI didn't post it as an angry response, just knew what kind of comments this would receive. Not sure why it got flagged, I wasn't saying this was some sort of excuse or that it should be rewritten, just found it interesting he included it. I'm sure he gets that comment a lot.
- ahoka 3y agoIt doesn’t help that the whole function is a huge computed goto mutating things, spanning multiple pages.
- notRobot 3y agoI have always been a huge fan of Daniel's writing. Such clear and honest writing explaining reasonings and perspectives.
- ChoHag 3y ago[flagged]
- pgeorgi 3y agoCome on, it starts out with a link to the CVE and a remark "While the advisory contains all the necessary details. I figured I would use a few additional words..." If you just want the gist of it, the article does everything it can to send you there.
- account-5 3y agoI have to say this is the best CVE writeups I've ever read. And I can only commend the author who is humble throughout despite authoring one of the cornerstone software products of the age. Much respect.
- gkhartman 3y agoIt still blows my mind that so many devices depend on a library largely written by a single person. The level of pressure must be insane and it shows in this bit: "Reading the code now it is impossible not to see the bug. Yes, it truly aches having to accept the fact that I did this mistake without noticing and that the flaw then remained undiscovered in code for 1315 days. I apologize. I am but a human." If Daniel happens to read this, thanks for your hard work, and I really don't think any apology is necessary. After all, the source was right there for any of us to read and review.
- nine_k 3y agohttps://xkcd.com/2347/ https://xkcd.com/2347/ had been true for many years before, and has been true for many years since it was published. Sometimes the "Don't fix it if it ain't broken" principle is interpreted as "don't support it until breaks", and leads to nasty surprises.
- avgcorrection 3y agoI think he'll be fine.
- BTinfinity 3y agoReminds me of how for a while OpenSSL was being developed and maintained by just two guys called Steve [0]. I think they've upgraded to a team of seven now at least [0] https://www.buzzfeed.com/chrisstokelwalker/the-internet-is-being-protected-by-two-guys-named-st https://www.buzzfeed.com/chrisstokelwalker/the-internet-is-b...
- pizza234 3y agoVery sensible conclusions about memory safety/memory-safe languages; excerpts follow. --- Yes, this family of flaws would have been impossible if curl had been written in a memory-safe language instead of C [...] The only approach in that direction I consider viable and sensible is to: - allow, use and support more dependencies written in memory-safe languages and - potentially and gradually replace parts of curl piecemeal, like with the introduction of hyper. Such development is however currently happening in a near glacial speed and shows with painful clarity the challenges involved. curl will remain written in C for the foreseeable future. Everyone not happy about this are of course welcome to roll up their sleeves and get working. Including the latest two CVEs reported for curl 8.4.0, the accumulated total says that 41% of the security vulnerabilities ever found in curl would likely not have happened should we have used a memory-safe language. But also: the rust language was not even a possibility for practical use for this purpose during the time in which we introduced maybe the first 80% of the C related problems. [...] We repeatedly run several static code analyzers on the code and none of them have spotted any problems in this function.
- deleted 3y ago[deleted]
- 4death4 3y agoReplacing parts of curl with Rust will not be possible. Most places want to build from source and don’t want to introduce a new compiler to the tool chain. And Rust supports a tiny subset of possible C targets.
- galangalalgol 3y agoNot wanting to introduce a new compiler is like wanting to introduce a new language. If you don't like the choice the package maintainer makes, roll up your sleeves. That said, we can mitigiate it using mrustc to generate c. Does anyone keep a list of all the targets libcurl is getting built for?
- agwa 3y agoShould the majority of us continue to bear the risks of memory unsafe code because of a small minority that uses exotic architectures or doesn't want to install a new compiler?
- dogben 3y agoif(!socks5_resolve_local && hostname_len > 255) { socks5_resolve_local = TRUE; } This is really a bad idea. For people who use anti-censorship tool to protect privacy, this can leak their identity through DNS.
- deleted 3y ago[deleted]
- vsnf 3y ago> I think it was downright wrong to switch mode like this, since the user asked for remote resolve curl should stick to that or fail. It is not even likely to work to just switch, even in “good” situations. Yes, the author shares your opinion.
- dig1 3y ago> Yes, this family of flaws would have been impossible if curl had been written in a memory-safe language instead of C It could have been impossible, but you never know if there is a way to escape language VM barriers. However, the author clearly ignored the DNS hostname limit stated in RFC1123 [1], which is hardcoded even in Java libraries. [1] https://www.rfc-editor.org/rfc/rfc1123 https://www.rfc-editor.org/rfc/rfc1123
- eesmith 3y agoCould you be more specific? All I saw was: Host software MUST handle host names of up to 63 characters and SHOULD handle host names of up to 255 characters. I don't see where the RFC sets a upper limit on host name size.
- dig1 3y ago6.1.3.5 Extensibility: The DNS defines domain name syntax very generally -- a string of labels each containing up to 63 8-bit octets, separated by dots, and with a maximum total of 255 octets. RFC1035 makes this more explicit [1]. [1] https://www.freesoft.org/CIE/RFC/1035/9.htm https://www.freesoft.org/CIE/RFC/1035/9.htm
- eesmith 3y agoThanks!
- matja 3y agoBy the wording of the RFC, accepting a hostname longer than 255 bytes is permissible, creating one is not.
- datenwolf 3y agomemory-safe language does not imply the use of a VM. It can just as well mean, that the compiler will attempt to execute a proof, that memory is never accessed out of bounds, without well defined ownership and within the lifetime of the underlying object. Which is what Rust does, for example. Of course it can also mean, that the compiler will then additionally add internal failure checks and safeguards at critical places (Rust does not do this, but it would be nice to have in systems where one might worry about in-register bit-flips (high radiation environments, like X-ray scanners), i.e. stuff not caught by – say – e.g. ECC memory).
- deng 3y agoThis is a great writeup. However, even after also reading the CVE, I'm still unsure about under which circumstances you would be affected. As far as I understand it, you are affected if you * use a SOCKS5 proxy, AND * you use the SOCKS5 proxy to resolve hostnames (which AFAICS is NOT the default), AND * the size of the buffer was changed from the default (100kb) to something below 65541 bytes, AND (EDIT: Not correct: 'libcurl' re-uses the download buffer for this, which by default is 16kB, however it is said that 'curl' itself sets it manually to 100kb UNLESS you use --limit-rate) * the SOCKS5 proxy is too slow to handle the request immediately (which however, as the CVE states, can usually be provoked if the attacker has control over the request rate). (EDIT: This is wrong, the CVE actually says "Typical server latency is likely "slow" enough to trigger this bug without an attacker needing to influence it by DoS or SOCKS server control.") So to me, the attack vector seems very small. Am I missing something?
- kiviuq 3y agoI'm trying to under the last comment > the SOCKS5 proxy is too slow to handle the request immediately the request seg faults immediately, no delays, no redirects root@1aac5e228e16:/build/curl-7.74.0# curl -vvv -x socks5h://host.docker.internal:9050 $(python3 -c "print(('A'10000), end='')") Trying 192.168.65.254:9050... * SOCKS5: server resolving disabled for hostnames of length > 255 [actual len=10000] * SOCKS5 connect to AAAAA... * Send failure: Bad file descriptor * Failed to send SOCKS5 connect request. Segmentation fault https://gist.github.com/xen0bit/0dccb11605abbeb6021963e2b1a811d3 https://gist.github.com/xen0bit/0dccb11605abbeb6021963e2b1a8...
- deng 3y agoYes sorry, I was wrong. The CVE says the opposite, I put an EDIT in the OP.
- deng 3y agoI'm confused why plain 'curl' would crash here: the CVE says that "The target buffer is the heap-based download buffer in libcurl that is reused for SOCKS negotiation before the transfer has started. The size of the buffer is 16kB by default, but can be set to different sizes by the application. The curl tool sets it to 102400 bytes by default - but it sets the buffer size to a smaller size if --limit-rate is set lower than 102400 bytes per second." I thought with a buffer of 100kB, this bug wouldn't trigger?
- Cloudef 3y agoQuite a bit of hype and drama for something that requires very specific condition to be exploited.
- palata 3y ago... and that is installed on billions on devices. I can imagine you quickly get very specific conditions here and there when something is deployed at that scale.
- faster_harder 3y agoCurl has had their share of issues with CVEs, I assume they want to show that they are taking actual security issues very seriously. See e.g. https://daniel.haxx.se/blog/2023/08/26/cve-2020-19909-is-everything-that-is-wrong-with-cves/ https://daniel.haxx.se/blog/2023/08/26/cve-2020-19909-is-eve...
- ynik 3y agoYesterday's Windows 10 security updates did not include a new curl version; the curl included with Windows is still 8.0.1.
- jeffrallen 3y agoThis is a long and interesting note which could be reduced to "this is why system libraries need to be either written in a safe language or proved correct". When one of the best, friendliest, and most transparent C programmers of our time is writing these posts, we need to pay attention.
- yjftsjthsd-h 3y ago> When one of the best, friendliest, and most transparent C programmers of our time is writing these posts, we need to pay attention. Given that said programmer wrote, > Such development is however currently happening in a near glacial speed and shows with painful clarity the challenges involved. curl will remain written in C for the foreseeable future. I'm curious what conclusion you'd like us to draw.
- andoaazczzz662 3y ago[dead]
- taspeotis 3y agohttps://hackerone.com/reports/2187833 https://hackerone.com/reports/2187833
- pstuart 3y agoMaybe I'm weird, but having a ternary that evaluates to ? TRUE : FALSE bugs me.
- jrpelkonen 3y agoSame. We could both be weird. Or not.