11 ms·

I have no problem with Curl being written in C (I'll take battle-tested C over experimental Rust) but this point seemed odd to me: >C is not the primary reason

by simias 10y ago

I have no problem with Curl being written in C (I'll take battle-tested C over experimental Rust) but this point seemed odd to me: >C is not the primary reason for our past vulnerabilities >There. The simple fact is that most of our past vulnerabilities happened because of logical mistakes in the code. Logical mistakes that aren’t really language bound and they would not be fixed simply by changing language. So I looked at https://curl.haxx.se/docs/security.html https://curl.haxx.se/docs/security.html #61 -> uninitialized random : libcurl's (new) internal function that returns a good 32bit random value was implemented poorly and overwrote the pointer instead of writing the value into the buffer the pointer pointed to. #60 -> printf floating point buffer overflow #57 -> cookie injection for other servers : The issue pertains to the function that loads cookies into memory, which reads the specified file into a fixed-size buffer in a line-by-line manner using the fgets() function. If an invocation of fgets() cannot read the whole line into the destination buffer due to it being too small, it truncates the output This one is arguably not really a failure of C itself, but I'd argue that Rust encourages a more robust error handling through its Options and Results when C tends to abuse "-1" and NULL return types that need careful checking and can't usually be enforced by the compiler. #55 -> OOB write via unchecked multiplication Rust has checked multiplication enabled by default in debug builds, and regardless of that the OOB wouldn't be possible. #54 -> Double free in curl_maprintf #53 -> Double free in krb5 code #52 -> glob parser write/read out of bound And I'll stop here, so far 7 out of 11 vulnerabilities would probably have been avoided with a safer language. Looks like the vast majority of these issues wouldn't have been possible in safe Rust.

eddieroger 10y ago
> Of course that leaves a share of problems that could’ve been avoided if we used another language. Buffer overflows, double frees and out of boundary reads etc, but the bulk of our security problems has not happened due to curl being written in C. He addressed all of those points in the second short paragraph. None of those are C vulnerabilities, they were mistakes made on the part of the developers, not the language. Avoidance of problems in a safer language doesn't mean when things happen, it's the language's fault.
- pjmlp 10y ago
  These are surely C vulnerabilities and contradict the statement regarding zero policy static analysis errors. #55 -> OOB write via unchecked multiplication #54 -> Double free in curl_maprintf #53 -> Double free in krb5 code #52 -> glob parser write/read out of bound
  - mikulas_florek 10y ago
    Double free is not necessary a C issue, but it can be also a program logic issue - I expect to have an object, but it's already deleted. So it's one of 1. object should not exist and the second free is incorrect 2. object should exist and the first free is incorrect 3. object existance is uncertain and the second free must somehow check that Although I can double-free only in unsafe languages, the wrong logic behind it can be the same in safe languages. It just have different consequences.
    - caconym_ 10y ago
      I don't want to jump on the "Rust fixes everything" train, but lifetimes, scope-based destructors and reference counted pointers seem like they help with this sort of thing beyond just preventing literal accesses of freed memory; they can make sure objects are around when you need them and that they're destroyed automatically when you don't anymore. Of course, you don't get it all for free; you have to wrestle with mutability and lifetimes, which can get hairy.
    - xorblurb 10y ago
      Of course it is a C issue, in the same sense that ALL logic errors are in the case you describe -- so either C issues do not exist or you did not manage to find the correct definition: for ex OOB access is caused by faulty program logic, and the consequences are dramatic in unsafe languages. That is an issue of the language, despite you being able to compute OOB indexes in any language. Same thing for double free; the language issue is that the result is catastrophic, not that you can write for ex faulty logic attempting a liberation too early, or an extra one. (Because in safe languages, the result is not as catastrophic as in unsafe languages). That is the whole concept of safety.
zzzcpan 10y ago
I think you are being too generous. All of those 11 vulnerabilities were caused by the language, its lack of memory safety, limited expressiveness, poor abstractions it encourages, etc.
- simias 10y ago
  That's not completely fair. It's true that there are some issues I've not counted that could be caused by the "limited expressiveness" of C. Notably the latest vulnerability, #62, which is caused by bogus error checking and would probably not have occurred in idiomatic Rust and could easily be blamed on C's terrible error handling ergonomics (or lack thereof). However for others it's not immediately obvious how a safer language would've helped, for instance #59: Win CE schannel cert wildcard matches too much This is clearly a logical error due to a badly implemented standard. There's no silvet bullet here.
  - kazinator 10y ago
    How a safer language helps with the non-obvious errors is: * makes code smaller with less distracting repeated boiler-plate, so things like that stand out more. * programmers waste less time on fighting language-related ergonomic pains, and so more of their attention is available to spot these errors. If your language is such that you shout "hooray" and pat yourself on the shoulder when it compiles and the code passes the address sanitizer and Valgrind and whatnot, then actual functional problems will slip under the rug. Permit me, also, to indulge in some argumentum ad lingua obscura: could we spot in a Brain##### program that some certificate wildcard matches too much? :)
    - rossy 10y ago
      It's easy to see the advantages of memory safety, since it eliminates an entire class of bugs that C programs tend to suffer from, but I think talking about language "ergonomics" is much more tenuous. One thing that C programmers tend to love about C is that it's simple. C doesn't have that many language constructs and that makes C programs easier to reason about, read and audit, which also makes it easier to find these non-obvious errors. A C programmer approaching an unfamiliar codebase can feel assured that it is made out of the same concise set of constructs as any other C program. On the other hand, modern languages like Rust, C# and Go are a lot more complicated. They have most of the language constructs of C, plus some extra, so not only do you have to understand the concepts in C, you also have to understand things like ownership, variance or pattern matching. Every new feature increases cognitive burden on the programmer. Adding something like exception handling, for example, means that control flow suddenly becomes more complex. Now, instead of only leaving at return statements, a function has a potential exit point anywhere it calls another function. "Smart" features that encourage terse code and reduce boilerplate can also result in code that is totally obtuse to anyone other than the person who wrote it. I'm not saying that C is at a sweet-spot for language complexity (Brainfuck is probably too simple, but OTOH it only has eight symbols!) I'm just saying that it's important to understand why a lot of developers really like C, and C is often praised for its simplicity. Any language that intends to replace C needs to understand why C programmers use C.
attractivechaos 10y ago
> Rust has checked multiplication enabled by default in debug builds, and regardless of that the OOB wouldn't be possible. That bug is only triggered by an unrealistic corner case (username longer than 512MB). Run-time check in debug build won't help unless you realize the possibility before hand and put a unit test for that. I am more interested in the second part of your comment: "the OOB wouldn't be possible". How does rust protect against such integer overflow caused by multiplication? Thanks in advance.
- anp 10y ago
  By default rust doesn't allow out of bounds indexing (indexing and iteration are checked, although bounds checks are often optimized out), you have to explicitly write unsafe code to read off the end of an array or vector.
  - caconym_ 10y ago
    Can you elaborate on bounds checks being optimized out? Is it only in certain situations where compiler can prove that they are completely unnecessary? Otherwise, they can't be relied on, so what's the point of having them at all?
    - steveklabnik 10y ago
      Yes, if the compiler can prove they're not needed, then it can remove the checks. But it has to prove it.
- jerf 10y ago
  "That bug is only triggered by an unrealistic corner case (username longer than 512MB)." What makes you call that an "unrealistic" case? You're probably imagining some sort of world where a randomly distributed set of usernames are sent to this function drawn from the distribution of "real usernames that people use". Since 0 of them are longer than 512MB, you intuitively assume a 0 probability of exploit. But in the security world, that's the wrong distribution. You have to assume a hostile, intelligent adversary. It isn't hard at all to construct a case of some web service allowing you to specify remote resources accessible with a username & password of your choosing (not corresponding to the username you're using to log in), an attacker specifying one of their own resources, reading the incoming headers to notice that you're using a vulnerable version of libcurl, and stuffing 512MB+exploit into the username field of your web app. If you don't add any other size restrictions between the attacker and the libcurl invocation, they may well pass it right in. (And your same intuition will lead you to not put any size restrictions there; "why would anybody have a multi-megabyte username?" You won't even have thought the question explicitly, you just won't put a length restriction on the field because it'll never even cross your mind.) By penetration standards, that doesn't even rise to the level of a challenge.
hzhou321 10y ago
> The simple fact is that most of our past vulnerabilities happened because of logical mistakes in the code. All bugs are logical errors at various scale.
astrobe_ 10y ago
> Looks like the vast majority of these issues wouldn't have been possible in safe Rust. Is it not where you're supposed to point to re-implementations or equivalents written in Rust?

by simias 10y ago

eddieroger 10y ago

> Of course that leaves a share of problems that could’ve been avoided if we used another language. Buffer overflows, double frees and out of boundary reads etc, but the bulk of our security problems has not happened due to curl being written in C. He addressed all of those points in the second short paragraph. None of those are C vulnerabilities, they were mistakes made on the part of the developers, not the language. Avoidance of problems in a safer language doesn't mean when things happen, it's the language's fault.

pjmlp 10y ago
These are surely C vulnerabilities and contradict the statement regarding zero policy static analysis errors. #55 -> OOB write via unchecked multiplication #54 -> Double free in curl_maprintf #53 -> Double free in krb5 code #52 -> glob parser write/read out of bound
- mikulas_florek 10y ago
  Double free is not necessary a C issue, but it can be also a program logic issue - I expect to have an object, but it's already deleted. So it's one of 1. object should not exist and the second free is incorrect 2. object should exist and the first free is incorrect 3. object existance is uncertain and the second free must somehow check that Although I can double-free only in unsafe languages, the wrong logic behind it can be the same in safe languages. It just have different consequences.
  - caconym_ 10y ago
    I don't want to jump on the "Rust fixes everything" train, but lifetimes, scope-based destructors and reference counted pointers seem like they help with this sort of thing beyond just preventing literal accesses of freed memory; they can make sure objects are around when you need them and that they're destroyed automatically when you don't anymore. Of course, you don't get it all for free; you have to wrestle with mutability and lifetimes, which can get hairy.
  - xorblurb 10y ago
    Of course it is a C issue, in the same sense that ALL logic errors are in the case you describe -- so either C issues do not exist or you did not manage to find the correct definition: for ex OOB access is caused by faulty program logic, and the consequences are dramatic in unsafe languages. That is an issue of the language, despite you being able to compute OOB indexes in any language. Same thing for double free; the language issue is that the result is catastrophic, not that you can write for ex faulty logic attempting a liberation too early, or an extra one. (Because in safe languages, the result is not as catastrophic as in unsafe languages). That is the whole concept of safety.

zzzcpan 10y ago

I think you are being too generous. All of those 11 vulnerabilities were caused by the language, its lack of memory safety, limited expressiveness, poor abstractions it encourages, etc.

simias 10y ago
That's not completely fair. It's true that there are some issues I've not counted that could be caused by the "limited expressiveness" of C. Notably the latest vulnerability, #62, which is caused by bogus error checking and would probably not have occurred in idiomatic Rust and could easily be blamed on C's terrible error handling ergonomics (or lack thereof). However for others it's not immediately obvious how a safer language would've helped, for instance #59: Win CE schannel cert wildcard matches too much This is clearly a logical error due to a badly implemented standard. There's no silvet bullet here.
- kazinator 10y ago
  How a safer language helps with the non-obvious errors is: * makes code smaller with less distracting repeated boiler-plate, so things like that stand out more. * programmers waste less time on fighting language-related ergonomic pains, and so more of their attention is available to spot these errors. If your language is such that you shout "hooray" and pat yourself on the shoulder when it compiles and the code passes the address sanitizer and Valgrind and whatnot, then actual functional problems will slip under the rug. Permit me, also, to indulge in some argumentum ad lingua obscura: could we spot in a Brain##### program that some certificate wildcard matches too much? :)
  - rossy 10y ago
    It's easy to see the advantages of memory safety, since it eliminates an entire class of bugs that C programs tend to suffer from, but I think talking about language "ergonomics" is much more tenuous. One thing that C programmers tend to love about C is that it's simple. C doesn't have that many language constructs and that makes C programs easier to reason about, read and audit, which also makes it easier to find these non-obvious errors. A C programmer approaching an unfamiliar codebase can feel assured that it is made out of the same concise set of constructs as any other C program. On the other hand, modern languages like Rust, C# and Go are a lot more complicated. They have most of the language constructs of C, plus some extra, so not only do you have to understand the concepts in C, you also have to understand things like ownership, variance or pattern matching. Every new feature increases cognitive burden on the programmer. Adding something like exception handling, for example, means that control flow suddenly becomes more complex. Now, instead of only leaving at return statements, a function has a potential exit point anywhere it calls another function. "Smart" features that encourage terse code and reduce boilerplate can also result in code that is totally obtuse to anyone other than the person who wrote it. I'm not saying that C is at a sweet-spot for language complexity (Brainfuck is probably too simple, but OTOH it only has eight symbols!) I'm just saying that it's important to understand why a lot of developers really like C, and C is often praised for its simplicity. Any language that intends to replace C needs to understand why C programmers use C.

attractivechaos 10y ago

> Rust has checked multiplication enabled by default in debug builds, and regardless of that the OOB wouldn't be possible. That bug is only triggered by an unrealistic corner case (username longer than 512MB). Run-time check in debug build won't help unless you realize the possibility before hand and put a unit test for that. I am more interested in the second part of your comment: "the OOB wouldn't be possible". How does rust protect against such integer overflow caused by multiplication? Thanks in advance.

anp 10y ago
By default rust doesn't allow out of bounds indexing (indexing and iteration are checked, although bounds checks are often optimized out), you have to explicitly write unsafe code to read off the end of an array or vector.
- caconym_ 10y ago
  Can you elaborate on bounds checks being optimized out? Is it only in certain situations where compiler can prove that they are completely unnecessary? Otherwise, they can't be relied on, so what's the point of having them at all?
  - steveklabnik 10y ago
    Yes, if the compiler can prove they're not needed, then it can remove the checks. But it has to prove it.
jerf 10y ago
"That bug is only triggered by an unrealistic corner case (username longer than 512MB)." What makes you call that an "unrealistic" case? You're probably imagining some sort of world where a randomly distributed set of usernames are sent to this function drawn from the distribution of "real usernames that people use". Since 0 of them are longer than 512MB, you intuitively assume a 0 probability of exploit. But in the security world, that's the wrong distribution. You have to assume a hostile, intelligent adversary. It isn't hard at all to construct a case of some web service allowing you to specify remote resources accessible with a username & password of your choosing (not corresponding to the username you're using to log in), an attacker specifying one of their own resources, reading the incoming headers to notice that you're using a vulnerable version of libcurl, and stuffing 512MB+exploit into the username field of your web app. If you don't add any other size restrictions between the attacker and the libcurl invocation, they may well pass it right in. (And your same intuition will lead you to not put any size restrictions there; "why would anybody have a multi-megabyte username?" You won't even have thought the question explicitly, you just won't put a length restriction on the field because it'll never even cross your mind.) By penetration standards, that doesn't even rise to the level of a challenge.

hzhou321 10y ago

> The simple fact is that most of our past vulnerabilities happened because of logical mistakes in the code. All bugs are logical errors at various scale.

astrobe_ 10y ago

> Looks like the vast majority of these issues wouldn't have been possible in safe Rust. Is it not where you're supposed to point to re-implementations or equivalents written in Rust?