35 ms·

This shouldn't have happened: A vulnerability postmortem

938 points by trulyrandom 5y ago

InfiniteRand 5y ago
Always place char arrays at the end of a struct - rule of thumb I heard somewhere, maybe from CERT-C That way if you do have memory corruption, the memory following your buffer is less predictable.
DantesKite 5y ago
This sounds like a very good argument for switching over to Rust.
- rfoo 5y ago
  Genuine question: How to switch codes written in 2003 to Rust?
  - varajelle 5y ago
    Maybe this is not so much about switching individual existing projects to Rust, but about switching the "industry". Some new projects are still written in C.
    - rfoo 5y ago
      Yeah, this makes sense. I'm optimistic and would like to say we're already half-way there. From my PoV very few new projects were written in C in recent years, except those inherently-C (their purpose was to call some other libraries written in C, or libc) and/or embedded (code size and portability requirements).
  - nix23 5y ago
    By work? It's pure risk-management, do you need it? Is it worth the potential risk/work?
  - howdydoo 5y ago
    Slowly and steadily.
  - jandrese 5y ago
    The same way you would write code written in 2021 over to Rust: by rewriting it from the ground up. Auto-translation won't work because Rust won't allow you to build it in the same way you would have built it in C. It requires a full up redesign of the code to follow the Rust development model.
    - masklinn 5y ago
      > Auto-translation won't work because Rust won't allow you to build it in the same way you would have built it in C. That is not entirely true, but if you translate the C code to Rust, you get C code, in Rust, with similar issues (or possibly worse). Of course the purpose would be to clean it up from there on, but it's unclear whether that's a better path than doing the conversion piecemeal by hand. The C2Rust people certainly seem to think so, but I don't know if there are good "client stories" about that path so far, whereas the manual approach does have some (e.g. librsvg), though it's not for the faint of heart.
      - lucb1e 5y ago
        > That is not entirely true, if you translate the C code to Rust, you get C code, in Rust, with similar issues (or possibly worse). thus it was basically true after all? Like, sure, Rust is turing-complete so you can simulate whatever C did and thus technically you can translate anything that C can do into Rust. But if it doesn't fix any problems, then have you really translated it into Rust?
        masklinn 5y ago
        > thus it was basically true after all? No? > Like, sure, Rust is turing-complete so you can simulate whatever C It's not simulating anything, and has nothing to do with turing completeness. > But if it doesn't fix any problems, then have you really translated it into Rust? Yeees? Unless your definition of "translated" has nothing to do with the word or the concept. You end up with a project full of rust code which builds using rust's toolchains. That sounds like a translation to me.
      - 5y ago
  - deleted 5y ago
    [deleted]
  - masklinn 5y ago
    librsvg 1.0 was in 2001. Federico Mena-Quintero started switching it to Rust in 2017 (well technically October 2016), the rewrite of the core was finished early 2019, though the test suite was only finished converting (aside from the C API tests) late 2020. So... carefully and slowly.
    - rfoo 5y ago
      Thanks, that's exactly what I'm seeking for. Before that I've never heard of projects which successfully did C to Rust transition, keeping its C API intact and could be used as a drop-in replacement. Glad to hear that there are already some success stories.
      - JeremyBanks 5y ago
        Not quite the same, but maybe of interest: https://daniel.haxx.se/blog/2020/10/09/rust-in-curl-with-hyper/ https://daniel.haxx.se/blog/2020/10/09/rust-in-curl-with-hyp...
  - deleted 5y ago
    [deleted]
- donkarma 5y ago
  More than just one memory safe language
  - graton 5y ago
    But Mozilla is already using Rust. They are a major proponent of Rust, so if they did switch to a memory safe language it would seem like Rust would be the most likely choice for them.
  - paavohtl 5y ago
    Not very many with 1) no garbage collection and 2) any meaningful open source adoption.
    - fulafel 5y ago
      This component (NSS) would work fine with GC.
- criddell 5y ago
  Rust has been around for more than a decade now and is still very niche. Evidently, it isn't a good enough argument.
  - lucb1e 5y ago
    I guess the ecosystem that might make a language attractive was not built overnight. I'm not sure looking at the popularity since an initial release is the best way to measure how good a language is for a particular purpose.
  - gostsamo 5y ago
    Actually, we are talking the creators of rust here. The same guys who were owning it with the idea to rewrite the entire browser in it. The more plausible reason might be that the rewrite to rust haven't advanced to this component yet.
    - criddell 5y ago
      Yeah, that could be. I was speaking about the wider development ecosystem. Rust is doing well in a few places and that's enough for it to survive and exist long term, or at least as long as Mozilla is relevant.
      - Arnavion 5y ago
        Mozilla's relevance hasn't mattered to Rust for a while.
        criddell 5y ago
        So if Mozilla decided to step back from Rust it wouldn't be a major blow to Rust? I was under the impression that they were still important.
        varajelle 5y ago
        They already stepped back a year ago. They fired most of the rust team. https://blog.mozilla.org/en/mozilla/changing-world-changing-mozilla/ https://blog.mozilla.org/en/mozilla/changing-world-changing-...
yjftsjthsd-h 5y ago
Ah; the title means "this shouldn't have happened [because the vender was in fact doing everything right]", not "this shouldn't have happened [because it's so stupid]".
- zelon88 5y ago
  Don't think for a minute this wasn't on purpose. Project Zero exists for the sole purpose of trashing and defacing Google competition. In the absence of actual process failure to report on they just resort to a disparagingly memorable title.
  - ziddoap 5y ago
    >This wasn’t a process failure, the vendor did everything right. Mozilla has a mature, world-class security team. They pioneered bug bounties, invest in memory safety, fuzzing and test coverage. Yep, definitely sounds like Project Zero is trashing Mozilla in this blog post.
    - zelon88 5y ago
      They checked for process failure didn't they? Nobody will remember that line. Everyone is going to remember the title.
      - ziddoap 5y ago
        The title doesn't name a vendor... So you'd have to read the article to see the vendor, where you would presumably read the line where they say they have a "world-class security team" among other praise. I don't like Google one bit, but my god these are some extraordinary hoops people are jumping through just so they can yell "Google's evil!".
        zelon88 5y ago
        I mean Google has this blog specifically to report on security vulnerabilities. That is literally like Volvo running a YT channel where they crash test cars from other companies and assess the damage to the dummy. "In the name of safety." I'm not the one stretching here.
edoceo 5y ago
Buffer Overflow is a classic right? (queue Rust enthusiasts)
- howdydoo 5y ago
  "This shouldn't have happened," says user of the only language where this regularly happens. https://www.theonion.com/no-way-to-prevent-this-says-only-nation-where-this-r-1846975039 https://www.theonion.com/no-way-to-prevent-this-says-only-na...
  - fulafel 5y ago
    Yep, Rust at best eliminates some already weak excuses to keep doing security critical parsing in the chainsaw-juggling traditon, when we've known better for 20+ years.
  - tialaramex 5y ago
    I've been meaning for some time to write one of these (with auto-generation whereas I believe The Onion actually has staff write a new one each time they run this article) for password database loss. [Because if you use Security Keys this entire problem dissipates. Your "password database" is just a bunch of public data, stealing it is maybe mildly embarrassing but has no impact on your users and is of no value to the "thieves"] But a Rust one for memory unsafety would be good too.
- petters 5y ago
  They are not wrong
- 2OEH8eoCRo0 5y ago
  It's wild how ahead of it's time Ada was.
- deleted 5y ago
  [deleted]
galadran 5y ago
The disclosure and test cases: https://www.openwall.com/lists/oss-security/2021/12/01/4 https://www.openwall.com/lists/oss-security/2021/12/01/4
lucb1e 5y ago
A title that actually describes the post, mostly paraphrasing the first paragraph: Reasons why this buffer overflow wasn't caught earlier despite doing all the right things And then to give those reasons: - "each component is fuzzed independently" ... "This fuzzer might have produced a SECKEYPublicKey that could have reached the vulnerable code, but as the result was never used to verify a signature, the bug could never be discovered." - "There is an arbitrary limit of 10000 bytes placed on fuzzed input. There is no such limit within NSS; many structures can exceed this size. This vulnerability demonstrates that errors happen at extremes" - "combined [fuzzer] coverage metrics [...]. This data proved misleading, as the vulnerable code is fuzzed extensively but by fuzzers that could not possibly generate a relevant input." The conclusion is, of course, to fix those problems if your code base also has them, but also "even extremely well-maintained C/C++ can have fatal, trivial mistakes".
- deleted 5y ago
  [deleted]
- jandrese 5y ago
  > - "There is an arbitrary limit of 10000 bytes placed on fuzzed input. There is no such limit within NSS; many structures can exceed this size. This vulnerability demonstrates that errors happen at extremes" This is the one that seemed short sighted to me. It's a completely arbitrary (and small!) limit that blinded the fuzzer to this very modest sized buffer overflow.
  - js2 5y ago
    The buffer holds 2K, so this limit alone which exceeds the buffer by 8K'ish didn't blind the fuzzer. It's not clear a larger input would've caught anything due to other "what went wrong" items, specifically "each component is fuzzed independently."
  - a-priori 5y ago
    The problem is that the search space grows (exponentially?) as you increase the fuzzer’s limit. So there’s a cost, and likely diminishing returns, to raising that limit.
slownews45 5y ago
Wow. We continue to be reminded that it's hard to write fully memory secure code in a language that is not memory secure? And by hard, I mean, very hard even for folks with lots of money and time and care (which is rare). My impression is that Apple's imessage and other stacks also have memory unsafe languages in the api/attack surface, and this has led to remote one click / no click type exploits. Is there a point at which someone says, hey, if it's very security sensitive write it in a language with a GC (golang?) or something crazy like rust? Or are C/C++ benefits just too high to ever give up? And similarly, that simplicity is a benefit (ie, BoringSSL etc has some value).
- jandrese 5y ago
  It's hard to fault a project written in 2003 for not using Go, Rust, Haskell, etc... It is also hard to convince people to do a ground up rewrite of code that is seemingly working fine.
  - zionic 5y ago
    >seemingly worked fine That’s just it though, it never was. That C/C++ code base is like a giant all-brick building on a fault line. It’s going to collapse eventually, and your users/the people inside will pay the price.
    - lelanthran 5y ago
      >>seemingly worked fine >That’s just it though, it never was. That C/C++ code base is like a giant all-brick building on a fault line. It’s going to collapse eventually, and your users/the people inside will pay the price. Sure, but everything is a trade-off[1]. In this particular case (and many others) no user appeared to pay any price, which tells me that the price is a spectrum ranging from 'Nothing' to 'FullyPwned' with graduations in between. Presumably the project will decide on what trade-off they are willing to make. [1] If I understand your comment correctly, you are saying that any C/C++ project has a 100% chance of a 'FullyPwned' outcome.
  - hwbehrens 5y ago
JulianMorrison 5y ago
Why isn't static analysis taint-checking the boundedness of data? Unbounded data should be flagged as unbounded and that flag should propagate through checking until it can be proven to be bounded.
mjw1007 5y ago
I think the main surprising thing here is that people are putting smallish arbitrary limits on the sizes of inputs that they let their fuzzer generate. With the benefit of a little hindsight, that does feel rather like saying "please try not to find any problems involving overflows".
- steve_adams_86 5y ago
  I agree. I haven’t done a lot of fuzzing, but my understanding is that this is how fuzzing can be helpful. Am I wrong? Or is it more complicated than that?
  - pornel 5y ago
    It's a trade-off. Larger input files may slow the fuzzing process, and therefore explore less of the problem space. You usually want to test many different kinds of inputs, not just more of the same. OTOH file formats often include sizes of fields, which a fuzzer will set to arbitrarily high values. This tests (some) handling of overly large inputs without files being actually that large.
oleganza 5y ago
Usually people say "oh, it's just another typical failure of writing in memory-unsafe C", but here's a slightly different angle: why is this common error is not happening under a single abstraction like "data structure that knows it size"? If C was allowing for such things, then 100000 programs would be using same 5-10 standard structures where the copy-and-overflow bug would be fixed already. Languages like Rust, of course, provide basic memory safety out of the box, but most importantly they also provide means to package unsafe code under safe API and debug it once and for all. And ecosystem of easy to use packages help reusing good code instead of reinventing your own binary buffers every single damn time, as it's usually done in C. So maybe it's not the unsafeness itself, but rather inability to build powerful reusable abstractions that plagues C? Everyone has to step on the same rake again and again and again.
- spullara 5y ago
  But performance! Rust and other languages with bounds checking go out of their way to not do it once it is proven that they don't need to. It would be hard to do that as a data structure.
  - oleganza 5y ago
    Well, here comes the type system, so your fancy data structure has zero cost. Rust recently got more support for const generics, so you could encode size bounds right in the types and skip unnecessary checks.
    - spullara 5y ago
      Oh that is what I was saying about Rust. I don't think that is possible in C, at least not without a huge amount of effort.
fulafel 5y ago
Good lesson also about how much our security relies on these largish ongoing fuzzing efforts, and makes you think what's going on at even larger fuzzing efforts that are less public.
thrdbndndn 5y ago
Kinda tangent, but when I was browsing NSS' repo ( https://hg.mozilla.org/projects/nss https://hg.mozilla.org/projects/nss or mirror: https://github.com/nss-dev/nss/commits/master https://github.com/nss-dev/nss/commits/master ) I found that the latest commit has a much older date (7 weeks ago) than the following ones. Why is that? (Sorry I don't know much about git other than push/pull.)
- spullara 5y ago
  Committed locally long ago and recently pushed?
- er4hn 5y ago
  The date of the commit is metadata which can be pushed later than it was made or even altered. If you look around you can find cute tools to alter your repo history and have the github commit history graph act as a pixelated billboard.
- account42 5y ago
  To expand on what others have said, the date shown is when the change was authored, which is not neccesarily the date when the commit object was created. In open source collaborative development, pathes are usually shared either old-style on mailing lists or review software (phabricator in this case it seems) as patches which include a date and then only applied in the repo once they are reviewed. You can also get non-monotonic authorship dates without leaving a git repo (and without manually overriding the date) by cherry-picking or rebasing commits onto different branches. Also, the first link is not a git repository but a mercurial one.
oxfeed65261 5y ago
I don’t understand why the “lessons learned” doesn’t recommend always* passing the destination buffer size (using memcpy_s or your own wrapper). It has been a long time since I wrote C++, but when I did this would have been instantly rejected in code review. *…with, I suppose, potential exceptions in performance-critical code when you control and trust the input; I don’t believe that this code qualifies on either count.
- rfoo 5y ago
  That's because these are "lessons learned" for how to catch these bugs, instead of "how to write more secure code". Because you can't.
  - jmull 5y ago
    You catch the bug by flagging the use of memcpy instead of something that takes the dest buffer size (like memcpy_s or whatever). It seems to me linters have been flagging this kind of thing since forever. This code is using a wrapper, "PORT_memcpy", so a default ruleset isn't going to flag it. So here I guess no one noticed PORT_memcpy == memcpy (or maybe noticed but didn't take the initiative to add a lint rule or deprecation entry or just created an issue to at least port existing code).
    - galangalalgol 5y ago
      was no one linting the wrapper? the static analysis tools we use wouldn't like memcpy_s either. It would create a finding to use an stl container probably.
- AnimalMuppet 5y ago
  Counterexample: msgrcv(). This expects you to not be passing raw buffers, but messages with a particular structure: a long mtype, to specify what type of message it is, and then a char (byte, since this is C) array that is the buffer that contains the rest of the message. You pass these structures to msgsnd() and msgrcv(), along with a size. But the size is the size of the buffer component of the structure, not the size of the structure as a whole. If you pass the size of the structure, it will read sizeof(long) more than your structure can hold. Been bit by that... So, just passing the size of the destination is something that you can still get wrong, in the case of data more complicated than just a single buffer. [Edit: You can also design an API to be very misleading, even if it has a length parameter...]
_wldu 5y ago
The sooner we can rewrite our programs in Go and Rust, the more secure we will be. Our shells, coreutils, mail readers and web browsers have to be written in safer languages.
- throwaway894345 5y ago
  Also, far, far easier to build than all of these C programs with their own bespoke build systems and implicit dependency management. The more of the software stack that can be built by mere mortals, the better.
  - rfoo 5y ago
    > far, far easier to build than all of these C programs One of my friends who work on AIX machines without direct Internet access does not share the same view, though.
    - throwaway894345 5y ago
      Why is indirect Internet access less of a problem for C than Rust/Go/etc? Seems like for modern systems, you just run a pre-populated caching proxy on your target and `cargo install` like you normally would. In C, you're manually checking versions and putting files in the right spot on disk for every stage of the build (this can be alleviated a bit if you can find pre-built binaries and so on, but even in the best case it's far behind "advanced" systems).
      - rfoo 5y ago
        > Why is indirect Internet access less of a problem for C than Rust/Go/etc? Because C codes tend to have less dependencies and shallow/more "clustered" dependency graph. To be fair, that's more or less due to dependency management being a 100% pain 0 fun experience.
        throwaway894345 5y ago
aidenn0 5y ago
I'm really curious why static analysis didn't catch this. If they weren't doing static analysis, I would probably have asserted that static analysis would catch this fairly easily. My guess would be too many (false?) positives on bounds-checking causing them to disable that check, but I can't be sure.
Veserv 5y ago
This absolutely should have happened. "Mature, world-class security teams" are, as a general rule, objectively terrible at creating products that meet any meaningful, objective definition of security. Remember a few years ago when Apple, the world's most valuable company, released a version of macOS that not only let you log into root with no password(!), but actually helpfully created a root account with the password supplied for the first person who tried to login to root[1]? Zerodium can purchase a vulnerability of similar severity to the one described in the article in Mozilla's premier product, Firefox, which undoubtedly has the best engineers at Mozilla and has had hundreds of millions if not billions spent on its development for $100k [2]. Even if we lowball the consulting rates for a skilled engineer at ~$500k, that means that we should expect a single, skilled engineer to, on average, find such a vulnerability with ~2 months of fulltime work otherwise the supply would have dried up. By no objective metric does taking 2 months of a single engineer's time to completely defeat the security of a widely used product constitute a meaningful, objective level of security. Even a two order of magnitude underestimation, literally 100x more than needed, still puts it in the range of a small team working for a year which still does not qualify as meaningful security. And, we can verify that this assessment is fairly consistent with the truth because we can ask basically any security professional if they believe a single person or a small team can completely breach their systems and they will invariably be scared shitless by the thought. The processes employed by the large, public, commercial tech companies that are viewed as leaders in security systemically produce software with security that is not only imperfect, it is not even good; it is terrible and is completely inadequate for any purpose where even just small scale criminal operations can be expected as seen by the rash of modern ransomware. Even the engineers who made these systems openly admit to this state of affairs [3] and many will even claim that it can not be made materially better. If the people making it are saying it is bad as a general rule, you should run away, fast. To achieve adequate protection against threat actors who actually act against these products would require not mere 100% improvements, it would require 10,000% or even 100,000% improvements in their processes. To give some perspective on that, people who tout Rust say that it if we switch to it we will remove the memory safety defects which are 70% of all security defects. If we use quantity of security defects as a proxy for security (which is an okay proxy to first order), that would require 6 successive switches to technologies each as much better than the last as people who like Rust say Rust is better than C++. That is how far away it all is, the security leaders do not need just a silver bullet, they need a whole silver revolver. In summary, a vulnerability like this is totally expected and not because they failed to have "world-class security" but because that is what "world-class security" actually means. [1] https://arstechnica.com/information-technology/2017/11/macos-bug-lets-you-log-in-as-admin-with-no-password-required/ https://arstechnica.com/information-technology/2017/11/macos... [2] https://zerodium.com/program.html https://zerodium.com/program.html (ZERODIUM Payouts for Desktops/Servers:Firefox RCE+LPE) [3] https://xkcd.com/2030/ https://xkcd.com/2030/ [4] https://www.zdnet.com/article/microsoft-70-percent-of-all-security-bugs-are-memory-safety-issues/ https://www.zdnet.com/article/microsoft-70-percent-of-all-se...
Jyaif 5y ago
For at least a decade, and in the teams I was in, C"++" written like this would not pass code review precisely because it is incredibly brittle.
- tialaramex 5y ago
  Uhuh. On cue a C++ programmer arrives to tell us that a true Scotsman wouldn't have introduced this bug... Where can we see "at least a decade" of this code you and your teams wrote?
Stampo00 5y ago
Why don't we have linters that would complain about copying memory without bounds checking?
deleted 5y ago
[deleted]
mleonhard 5y ago
What went wrong - Issue #0: The library was not re-written in a language that prevents undefined behavior (UB).
- tialaramex 5y ago
  I don't think there are any general purpose programming languages with decent performance which outright "prevent undefined behaviour" in something like NSS. Rust, for example, does not. safe Rust doesn't have undefined behaviour but of course you can (and a large project like this will) use unsafe Rust and then you need the same precautions for that code. This sharply reduces your exposure if you're doing a decent job - and is therefore worthwhile but it is not a silver bullet. Outright preventing undefined behaviour is hard. Java outlaws it, but probably not successfully (I believe it's a bug if your Java VM exhibits undefined behaviour, but you may find that unless it was trivial to exploit it goes in the pile of known bugs and nobody is jumping up and down to fix it). Go explains in its deeper documentation that concurrent Go is unsafe (this is one of the places where Rust is safer, safe Rust is still safe concurrently). Something like WUFFS prevents undefined behaviour and has excellent performance but it has a deliberately limited domain rather than being a general purpose language. Perhaps a language like WUFFS should exist for much of the work NSS does. But Rust does exist, it was created at Mozilla where NSS lives, and NSS wasn't rewritten in Rust so why should we expect it to be rewritten in this hypothetical Wrangling Untrusted Cryptographic Data Safely language?
  - mleonhard 5y ago
    > I don't think there are any general purpose programming languages with decent performance which outright "prevent undefined behaviour" in something like NSS. Rust, for example, does not. You know what I meant by "a language that prevents UB". Your comment argues semantics. That's not nice. Please stop. > safe Rust doesn't have undefined behaviour but of course you can (and a large project like this will) use unsafe Rust and then you need the same precautions for that code. How can that can be true? A large Rust project uses unsafe only because its authors don't care enough. Instead of spending the effort to make the safe code fast enough, they resort to using `unsafe`. It's the same reason that people add code without tests or add APIs without docs. A special purpose language for writing cryptography code is a terrible idea. Once we have a language suitable for handling untrusted data, we should make it good enough to use for everything and then use it for everything. I have put some effort into making safe Rust usable for everything. I wrote a safe regex library [0] and a safe async library [1]. I plan to put more effort into these and other libraries. For example, I am working on safe Rust TLS and HTTP libraries. Eventually, safe Rust will be production quality and fast enough for most use-cases. [0] https://crates.io/crates/safe-regex https://crates.io/crates/safe-regex [1] https://crates.io/crates/safina https://crates.io/crates/safina
deleted 5y ago
[deleted]
nielsole 5y ago
> Issue #2 Arbitrary size limits.[...] > A reasonable choice might be 2^24-1 bytes, the largest possible certificate How does one treat untrusted input whose length might exceed available memory? I am working on a patch for a jwks implementation which does not even have upper bounds in the spec. Accepting any valid input until OOMing seems like a suboptimal solution.
- duped 5y ago
  In a sense, reducing the error case to the physical limitations of a device is a perfectly "optimal" solution
- saagarjha 5y ago
  Generally, taint tracking.
rurban 5y ago
Now take a deep look at the POSIX C standard, Annex K. The bounds checked extensions. Using these would have definitely avoided the problem. memcpy_s requires the size of dest to be defined. The applause goes to the glibc maintainers, who still think they are above that.
- ndesaulniers 5y ago
  Doesn't memcpy_s not take into account the sources size though? You could still read past the end of an object with it, right?
  - rurban 5y ago
    nope, it checks both. How could you call it secure without checking both sizes? memcpy_s(dest,dmax,src,slen) and my implementation safeclib even at compile-time, similar to glibc's FORTIFY.
- loeg 5y ago
  Annex K is pretty awful[1]. There are plenty of fine solutions here in hindsight, but glibc adopting Annex K isn’t one of them. NSS has a plethora of build targets, including Windows - which does not implement Annex K either (despite inspiring it). [1]: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1967.htm http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1967.htm
  - rurban 5y ago
    Wrong. First it's good, not awful. Awful are only the ones not using it. Second, Windows implements Annex K as only major provider. The others are some minor embedded targets, plus Android Bionic recently. Implementations, such as safeclib, are cross platform. you can use it everywhere. Security and crypto people per se don't use it (incompetence or not invented here), but security aware people, such as on embedded or in the industry.
albntomat0 5y ago
Since this comes up whenever there is a Project Zero article, here is a summary I made in summer 2020 on the distribution of the bugs they find/report: Since this always comes up, here's an overview I made several weeks ago about where Project Zero focuses their efforts: All counts are rough numbers. Project zero posts: Google: 24 Apple: 28 Microsoft: 36 I was curious, so I poked around the project zero bug tracker to try to find ground truth about their bug reporting: https://bugs.chromium.org/p/project-zero/issues/list https://bugs.chromium.org/p/project-zero/issues/list For all issues, including closed: product=Android returns 81 results product=iOS returns 58 vendor=Apple returns 380 vendor=Google returns 145 (bugs in Samsung's Android kernel,etc. are tracked separately) vendor=Linux return 54 To be fair, a huge number of things make this not an even comparison, including the underlying bug rate, different products and downstream Android vendors being tracked separately. Also, # bugs found != which ones they choose to write about.
- deleted 5y ago
  [deleted]
- gausswho 5y ago
  I'm not clear. What do these varying counts imply to you?
  - nova22033 5y ago
    Because someone will invariably accuse project zero of being hostile to google's competitors.
- sangnoir 5y ago
  > I made several weeks ago about where Project Zero focuses their efforts Are you sure the numbers track where they put effort? I can think of a couple of confounding factors (including P0 methodologies, and number of low-hanging[1] bugs in target products) 1. Relatively speaking
  - albntomat0 5y ago
    I absolutely agree that there are numerous reasons why the numbers are not equal. My sole reason for posting is the "P0 is a hit squad against Apple/Mozzilla/Microsoft" comments that come up whenever their blog posts end up on HN.
nomoreusernames 5y ago
so who wants to tell linus to rewrite everything in rust?
jmull 5y ago
To me, PORT_Memcpy is one problem here. There are two buffers and one size -- the amount of memory to copy. There should be PORT_Memcpy2(pDest, destSize, pSource, numBytesToCopy) (or whatever you want to call it) which at least prompts the programmer to account for the size destination buffer. Then flag all calls to PORT_Memcpy and at least make a dev look at it. (Same for the various similar functions like strcpy, etc.)
- twodayslate 5y ago
  Of course it would just end up being PORT_Memcpy2(cx->u.buffer, sigLen, sig->data, sigLen);
  - jmull 5y ago
    Someone could do that, but the point of having the dest buffer size is to at least give the programmer a chance to try to get it right. I also wonder if a linter could notice that the dest buffer size passed isn’t the actual size of the buffer. (That leads the the next problem in the code, if you look at the definition of that buffer, so that’s good.)
- ndesaulniers 5y ago
  numBytesToCpy != sourceSize Otherwise that's a potential read out of bounds.
Eduard 5y ago
How realistic is it that this vulnerability can be exploited for $BAD_THINGS? https://www.mozilla.org/en-US/security/advisories/mfsa2021-51/ https://www.mozilla.org/en-US/security/advisories/mfsa2021-5... notes "This vulnerability does NOT impact Mozilla Firefox. However, email clients and PDF viewers that use NSS for signature verification, such as Thunderbird, LibreOffice, Evolution and Evince are believed to be impacted.".
sitkack 5y ago
All ASN1 parsers need to get replaced with safe Rust code, full stop.
jlakjsdlkjfowif 5y ago
This is what happens when you let people write C that don't know what they are doing. Probably a new grad ?
aoetalks 5y ago
When will we switch to memory safe (but reasonably performant) languages like Go/Rust/C#? Performance critical sections could remain in C/C++, but MOST code doesn’t need the kind of performance C/C++ provides. Hopefully C/C++ will go the way of assembly: present in TINY doses.
- shp0ngle 5y ago
  I don't think you need to explain to Mozilla about Just Rewriting It to Rust
  - lmm 5y ago
    Didn't they recently fire the Rust team?
    - steveklabnik 5y ago
      While they did let go the folks who were working on Rust (see the other comments in this thread) they are still members of the Rust foundation, and folks still write code in Rust at Mozilla. For example you have this from last week https://groups.google.com/a/mozilla.org/g/dev-platform/c/dVUHF0JY5G4/m/c9VyDijbAwAJ https://groups.google.com/a/mozilla.org/g/dev-platform/c/dVU...
brundolf 5y ago
Which end-user applications are affected? And what would an attacker have to do to exploit this in the wild?
DikshaMA 5y ago
Packers and Movers Bangalore - Reliable and Verified Household Shifting Service Providers Give Reasonable ###Packers and Movers Charges. Cheap and Best Office Relocation Compare Quotation for Assurance for Local and Domestic House Shifting and Get estimates today to save upto 20%, **Read Customer Reviews - @ https://packersmoversbangalore.in/ https://packersmoversbangalore.in/
sneak 5y ago
You'd think that in the wake of "goto fail;" the organization that pioneered rust would have rewritten their core TLS certificate checking library in rust by now, hacker news tropes notwithstanding.
throwaway5371 5y ago
i love the fact that people can read and write whatever they want wherever they want go and rust make things a bit boring, c makes me feel like a kid of course the price for freedom is security haha

938 points by trulyrandom 5y ago

Always place char arrays at the end of a struct - rule of thumb I heard somewhere, maybe from CERT-C That way if you do have memory corruption, the memory following your buffer is less predictable.

DantesKite 5y ago

This sounds like a very good argument for switching over to Rust.

rfoo 5y ago
Genuine question: How to switch codes written in 2003 to Rust?
- varajelle 5y ago
  Maybe this is not so much about switching individual existing projects to Rust, but about switching the "industry". Some new projects are still written in C.
  - rfoo 5y ago
    Yeah, this makes sense. I'm optimistic and would like to say we're already half-way there. From my PoV very few new projects were written in C in recent years, except those inherently-C (their purpose was to call some other libraries written in C, or libc) and/or embedded (code size and portability requirements).
- nix23 5y ago
  By work? It's pure risk-management, do you need it? Is it worth the potential risk/work?
- howdydoo 5y ago
  Slowly and steadily.
- jandrese 5y ago
  The same way you would write code written in 2021 over to Rust: by rewriting it from the ground up. Auto-translation won't work because Rust won't allow you to build it in the same way you would have built it in C. It requires a full up redesign of the code to follow the Rust development model.
  - masklinn 5y ago
    > Auto-translation won't work because Rust won't allow you to build it in the same way you would have built it in C. That is not entirely true, but if you translate the C code to Rust, you get C code, in Rust, with similar issues (or possibly worse). Of course the purpose would be to clean it up from there on, but it's unclear whether that's a better path than doing the conversion piecemeal by hand. The C2Rust people certainly seem to think so, but I don't know if there are good "client stories" about that path so far, whereas the manual approach does have some (e.g. librsvg), though it's not for the faint of heart.
    - lucb1e 5y ago
      > That is not entirely true, if you translate the C code to Rust, you get C code, in Rust, with similar issues (or possibly worse). thus it was basically true after all? Like, sure, Rust is turing-complete so you can simulate whatever C did and thus technically you can translate anything that C can do into Rust. But if it doesn't fix any problems, then have you really translated it into Rust?
      - masklinn 5y ago
        > thus it was basically true after all? No? > Like, sure, Rust is turing-complete so you can simulate whatever C It's not simulating anything, and has nothing to do with turing completeness. > But if it doesn't fix any problems, then have you really translated it into Rust? Yeees? Unless your definition of "translated" has nothing to do with the word or the concept. You end up with a project full of rust code which builds using rust's toolchains. That sounds like a translation to me.
    - 5y ago
- deleted 5y ago
  [deleted]
- masklinn 5y ago
  librsvg 1.0 was in 2001. Federico Mena-Quintero started switching it to Rust in 2017 (well technically October 2016), the rewrite of the core was finished early 2019, though the test suite was only finished converting (aside from the C API tests) late 2020. So... carefully and slowly.
  - rfoo 5y ago
    Thanks, that's exactly what I'm seeking for. Before that I've never heard of projects which successfully did C to Rust transition, keeping its C API intact and could be used as a drop-in replacement. Glad to hear that there are already some success stories.
    - JeremyBanks 5y ago
      Not quite the same, but maybe of interest: https://daniel.haxx.se/blog/2020/10/09/rust-in-curl-with-hyper/ https://daniel.haxx.se/blog/2020/10/09/rust-in-curl-with-hyp...
- deleted 5y ago
  [deleted]
donkarma 5y ago
More than just one memory safe language
- graton 5y ago
  But Mozilla is already using Rust. They are a major proponent of Rust, so if they did switch to a memory safe language it would seem like Rust would be the most likely choice for them.
- paavohtl 5y ago
  Not very many with 1) no garbage collection and 2) any meaningful open source adoption.
  - fulafel 5y ago
    This component (NSS) would work fine with GC.
criddell 5y ago
Rust has been around for more than a decade now and is still very niche. Evidently, it isn't a good enough argument.
- lucb1e 5y ago
  I guess the ecosystem that might make a language attractive was not built overnight. I'm not sure looking at the popularity since an initial release is the best way to measure how good a language is for a particular purpose.
- gostsamo 5y ago
  Actually, we are talking the creators of rust here. The same guys who were owning it with the idea to rewrite the entire browser in it. The more plausible reason might be that the rewrite to rust haven't advanced to this component yet.
  - criddell 5y ago
    Yeah, that could be. I was speaking about the wider development ecosystem. Rust is doing well in a few places and that's enough for it to survive and exist long term, or at least as long as Mozilla is relevant.
    - Arnavion 5y ago
      Mozilla's relevance hasn't mattered to Rust for a while.
      - criddell 5y ago
        So if Mozilla decided to step back from Rust it wouldn't be a major blow to Rust? I was under the impression that they were still important.
        varajelle 5y ago
        They already stepped back a year ago. They fired most of the rust team. https://blog.mozilla.org/en/mozilla/changing-world-changing-mozilla/ https://blog.mozilla.org/en/mozilla/changing-world-changing-...

yjftsjthsd-h 5y ago

Ah; the title means "this shouldn't have happened [because the vender was in fact doing everything right]", not "this shouldn't have happened [because it's so stupid]".

zelon88 5y ago
Don't think for a minute this wasn't on purpose. Project Zero exists for the sole purpose of trashing and defacing Google competition. In the absence of actual process failure to report on they just resort to a disparagingly memorable title.
- ziddoap 5y ago
  >This wasn’t a process failure, the vendor did everything right. Mozilla has a mature, world-class security team. They pioneered bug bounties, invest in memory safety, fuzzing and test coverage. Yep, definitely sounds like Project Zero is trashing Mozilla in this blog post.
  - zelon88 5y ago
    They checked for process failure didn't they? Nobody will remember that line. Everyone is going to remember the title.
    - ziddoap 5y ago
      The title doesn't name a vendor... So you'd have to read the article to see the vendor, where you would presumably read the line where they say they have a "world-class security team" among other praise. I don't like Google one bit, but my god these are some extraordinary hoops people are jumping through just so they can yell "Google's evil!".
      - zelon88 5y ago
        I mean Google has this blog specifically to report on security vulnerabilities. That is literally like Volvo running a YT channel where they crash test cars from other companies and assess the damage to the dummy. "In the name of safety." I'm not the one stretching here.

edoceo 5y ago

Buffer Overflow is a classic right? (queue Rust enthusiasts)

howdydoo 5y ago
"This shouldn't have happened," says user of the only language where this regularly happens. https://www.theonion.com/no-way-to-prevent-this-says-only-nation-where-this-r-1846975039 https://www.theonion.com/no-way-to-prevent-this-says-only-na...
- fulafel 5y ago
  Yep, Rust at best eliminates some already weak excuses to keep doing security critical parsing in the chainsaw-juggling traditon, when we've known better for 20+ years.
- tialaramex 5y ago
  I've been meaning for some time to write one of these (with auto-generation whereas I believe The Onion actually has staff write a new one each time they run this article) for password database loss. [Because if you use Security Keys this entire problem dissipates. Your "password database" is just a bunch of public data, stealing it is maybe mildly embarrassing but has no impact on your users and is of no value to the "thieves"] But a Rust one for memory unsafety would be good too.
petters 5y ago
They are not wrong
2OEH8eoCRo0 5y ago
It's wild how ahead of it's time Ada was.
deleted 5y ago
[deleted]

galadran 5y ago

The disclosure and test cases: https://www.openwall.com/lists/oss-security/2021/12/01/4 https://www.openwall.com/lists/oss-security/2021/12/01/4

lucb1e 5y ago

A title that actually describes the post, mostly paraphrasing the first paragraph: Reasons why this buffer overflow wasn't caught earlier despite doing all the right things And then to give those reasons: - "each component is fuzzed independently" ... "This fuzzer might have produced a SECKEYPublicKey that could have reached the vulnerable code, but as the result was never used to verify a signature, the bug could never be discovered." - "There is an arbitrary limit of 10000 bytes placed on fuzzed input. There is no such limit within NSS; many structures can exceed this size. This vulnerability demonstrates that errors happen at extremes" - "combined [fuzzer] coverage metrics [...]. This data proved misleading, as the vulnerable code is fuzzed extensively but by fuzzers that could not possibly generate a relevant input." The conclusion is, of course, to fix those problems if your code base also has them, but also "even extremely well-maintained C/C++ can have fatal, trivial mistakes".

deleted 5y ago
[deleted]
jandrese 5y ago
> - "There is an arbitrary limit of 10000 bytes placed on fuzzed input. There is no such limit within NSS; many structures can exceed this size. This vulnerability demonstrates that errors happen at extremes" This is the one that seemed short sighted to me. It's a completely arbitrary (and small!) limit that blinded the fuzzer to this very modest sized buffer overflow.
- js2 5y ago
  The buffer holds 2K, so this limit alone which exceeds the buffer by 8K'ish didn't blind the fuzzer. It's not clear a larger input would've caught anything due to other "what went wrong" items, specifically "each component is fuzzed independently."
- a-priori 5y ago
  The problem is that the search space grows (exponentially?) as you increase the fuzzer’s limit. So there’s a cost, and likely diminishing returns, to raising that limit.

slownews45 5y ago

Wow. We continue to be reminded that it's hard to write fully memory secure code in a language that is not memory secure? And by hard, I mean, very hard even for folks with lots of money and time and care (which is rare). My impression is that Apple's imessage and other stacks also have memory unsafe languages in the api/attack surface, and this has led to remote one click / no click type exploits. Is there a point at which someone says, hey, if it's very security sensitive write it in a language with a GC (golang?) or something crazy like rust? Or are C/C++ benefits just too high to ever give up? And similarly, that simplicity is a benefit (ie, BoringSSL etc has some value).

jandrese 5y ago
It's hard to fault a project written in 2003 for not using Go, Rust, Haskell, etc... It is also hard to convince people to do a ground up rewrite of code that is seemingly working fine.
- zionic 5y ago
  >seemingly worked fine That’s just it though, it never was. That C/C++ code base is like a giant all-brick building on a fault line. It’s going to collapse eventually, and your users/the people inside will pay the price.
  - lelanthran 5y ago
    >>seemingly worked fine >That’s just it though, it never was. That C/C++ code base is like a giant all-brick building on a fault line. It’s going to collapse eventually, and your users/the people inside will pay the price. Sure, but everything is a trade-off[1]. In this particular case (and many others) no user appeared to pay any price, which tells me that the price is a spectrum ranging from 'Nothing' to 'FullyPwned' with graduations in between. Presumably the project will decide on what trade-off they are willing to make. [1] If I understand your comment correctly, you are saying that any C/C++ project has a 100% chance of a 'FullyPwned' outcome.
- hwbehrens 5y ago

JulianMorrison 5y ago

Why isn't static analysis taint-checking the boundedness of data? Unbounded data should be flagged as unbounded and that flag should propagate through checking until it can be proven to be bounded.

mjw1007 5y ago

I think the main surprising thing here is that people are putting smallish arbitrary limits on the sizes of inputs that they let their fuzzer generate. With the benefit of a little hindsight, that does feel rather like saying "please try not to find any problems involving overflows".

steve_adams_86 5y ago
I agree. I haven’t done a lot of fuzzing, but my understanding is that this is how fuzzing can be helpful. Am I wrong? Or is it more complicated than that?
- pornel 5y ago
  It's a trade-off. Larger input files may slow the fuzzing process, and therefore explore less of the problem space. You usually want to test many different kinds of inputs, not just more of the same. OTOH file formats often include sizes of fields, which a fuzzer will set to arbitrarily high values. This tests (some) handling of overly large inputs without files being actually that large.

oleganza 5y ago

Usually people say "oh, it's just another typical failure of writing in memory-unsafe C", but here's a slightly different angle: why is this common error is not happening under a single abstraction like "data structure that knows it size"? If C was allowing for such things, then 100000 programs would be using same 5-10 standard structures where the copy-and-overflow bug would be fixed already. Languages like Rust, of course, provide basic memory safety out of the box, but most importantly they also provide means to package unsafe code under safe API and debug it once and for all. And ecosystem of easy to use packages help reusing good code instead of reinventing your own binary buffers every single damn time, as it's usually done in C. So maybe it's not the unsafeness itself, but rather inability to build powerful reusable abstractions that plagues C? Everyone has to step on the same rake again and again and again.

spullara 5y ago
But performance! Rust and other languages with bounds checking go out of their way to not do it once it is proven that they don't need to. It would be hard to do that as a data structure.
- oleganza 5y ago
  Well, here comes the type system, so your fancy data structure has zero cost. Rust recently got more support for const generics, so you could encode size bounds right in the types and skip unnecessary checks.
  - spullara 5y ago
    Oh that is what I was saying about Rust. I don't think that is possible in C, at least not without a huge amount of effort.

fulafel 5y ago

Good lesson also about how much our security relies on these largish ongoing fuzzing efforts, and makes you think what's going on at even larger fuzzing efforts that are less public.

thrdbndndn 5y ago

Kinda tangent, but when I was browsing NSS' repo ( https://hg.mozilla.org/projects/nss https://hg.mozilla.org/projects/nss or mirror: https://github.com/nss-dev/nss/commits/master https://github.com/nss-dev/nss/commits/master ) I found that the latest commit has a much older date (7 weeks ago) than the following ones. Why is that? (Sorry I don't know much about git other than push/pull.)

spullara 5y ago
Committed locally long ago and recently pushed?
er4hn 5y ago
The date of the commit is metadata which can be pushed later than it was made or even altered. If you look around you can find cute tools to alter your repo history and have the github commit history graph act as a pixelated billboard.
account42 5y ago
To expand on what others have said, the date shown is when the change was authored, which is not neccesarily the date when the commit object was created. In open source collaborative development, pathes are usually shared either old-style on mailing lists or review software (phabricator in this case it seems) as patches which include a date and then only applied in the repo once they are reviewed. You can also get non-monotonic authorship dates without leaving a git repo (and without manually overriding the date) by cherry-picking or rebasing commits onto different branches. Also, the first link is not a git repository but a mercurial one.

oxfeed65261 5y ago

I don’t understand why the “lessons learned” doesn’t recommend always* passing the destination buffer size (using memcpy_s or your own wrapper). It has been a long time since I wrote C++, but when I did this would have been instantly rejected in code review. *…with, I suppose, potential exceptions in performance-critical code when you control and trust the input; I don’t believe that this code qualifies on either count.

rfoo 5y ago
That's because these are "lessons learned" for how to catch these bugs, instead of "how to write more secure code". Because you can't.
- jmull 5y ago
  You catch the bug by flagging the use of memcpy instead of something that takes the dest buffer size (like memcpy_s or whatever). It seems to me linters have been flagging this kind of thing since forever. This code is using a wrapper, "PORT_memcpy", so a default ruleset isn't going to flag it. So here I guess no one noticed PORT_memcpy == memcpy (or maybe noticed but didn't take the initiative to add a lint rule or deprecation entry or just created an issue to at least port existing code).
  - galangalalgol 5y ago
    was no one linting the wrapper? the static analysis tools we use wouldn't like memcpy_s either. It would create a finding to use an stl container probably.
AnimalMuppet 5y ago
Counterexample: msgrcv(). This expects you to not be passing raw buffers, but messages with a particular structure: a long mtype, to specify what type of message it is, and then a char (byte, since this is C) array that is the buffer that contains the rest of the message. You pass these structures to msgsnd() and msgrcv(), along with a size. But the size is the size of the buffer component of the structure, not the size of the structure as a whole. If you pass the size of the structure, it will read sizeof(long) more than your structure can hold. Been bit by that... So, just passing the size of the destination is something that you can still get wrong, in the case of data more complicated than just a single buffer. [Edit: You can also design an API to be very misleading, even if it has a length parameter...]

_wldu 5y ago

The sooner we can rewrite our programs in Go and Rust, the more secure we will be. Our shells, coreutils, mail readers and web browsers have to be written in safer languages.

throwaway894345 5y ago
Also, far, far easier to build than all of these C programs with their own bespoke build systems and implicit dependency management. The more of the software stack that can be built by mere mortals, the better.
- rfoo 5y ago
  > far, far easier to build than all of these C programs One of my friends who work on AIX machines without direct Internet access does not share the same view, though.
  - throwaway894345 5y ago
    Why is indirect Internet access less of a problem for C than Rust/Go/etc? Seems like for modern systems, you just run a pre-populated caching proxy on your target and `cargo install` like you normally would. In C, you're manually checking versions and putting files in the right spot on disk for every stage of the build (this can be alleviated a bit if you can find pre-built binaries and so on, but even in the best case it's far behind "advanced" systems).
    - rfoo 5y ago
      > Why is indirect Internet access less of a problem for C than Rust/Go/etc? Because C codes tend to have less dependencies and shallow/more "clustered" dependency graph. To be fair, that's more or less due to dependency management being a 100% pain 0 fun experience.
      - throwaway894345 5y ago

aidenn0 5y ago

I'm really curious why static analysis didn't catch this. If they weren't doing static analysis, I would probably have asserted that static analysis would catch this fairly easily. My guess would be too many (false?) positives on bounds-checking causing them to disable that check, but I can't be sure.

Veserv 5y ago

This absolutely should have happened. "Mature, world-class security teams" are, as a general rule, objectively terrible at creating products that meet any meaningful, objective definition of security. Remember a few years ago when Apple, the world's most valuable company, released a version of macOS that not only let you log into root with no password(!), but actually helpfully created a root account with the password supplied for the first person who tried to login to root[1]? Zerodium can purchase a vulnerability of similar severity to the one described in the article in Mozilla's premier product, Firefox, which undoubtedly has the best engineers at Mozilla and has had hundreds of millions if not billions spent on its development for $100k [2]. Even if we lowball the consulting rates for a skilled engineer at ~$500k, that means that we should expect a single, skilled engineer to, on average, find such a vulnerability with ~2 months of fulltime work otherwise the supply would have dried up. By no objective metric does taking 2 months of a single engineer's time to completely defeat the security of a widely used product constitute a meaningful, objective level of security. Even a two order of magnitude underestimation, literally 100x more than needed, still puts it in the range of a small team working for a year which still does not qualify as meaningful security. And, we can verify that this assessment is fairly consistent with the truth because we can ask basically any security professional if they believe a single person or a small team can completely breach their systems and they will invariably be scared shitless by the thought. The processes employed by the large, public, commercial tech companies that are viewed as leaders in security systemically produce software with security that is not only imperfect, it is not even good; it is terrible and is completely inadequate for any purpose where even just small scale criminal operations can be expected as seen by the rash of modern ransomware. Even the engineers who made these systems openly admit to this state of affairs [3] and many will even claim that it can not be made materially better. If the people making it are saying it is bad as a general rule, you should run away, fast. To achieve adequate protection against threat actors who actually act against these products would require not mere 100% improvements, it would require 10,000% or even 100,000% improvements in their processes. To give some perspective on that, people who tout Rust say that it if we switch to it we will remove the memory safety defects which are 70% of all security defects. If we use quantity of security defects as a proxy for security (which is an okay proxy to first order), that would require 6 successive switches to technologies each as much better than the last as people who like Rust say Rust is better than C++. That is how far away it all is, the security leaders do not need just a silver bullet, they need a whole silver revolver. In summary, a vulnerability like this is totally expected and not because they failed to have "world-class security" but because that is what "world-class security" actually means. [1] https://arstechnica.com/information-technology/2017/11/macos-bug-lets-you-log-in-as-admin-with-no-password-required/ https://arstechnica.com/information-technology/2017/11/macos... [2] https://zerodium.com/program.html https://zerodium.com/program.html (ZERODIUM Payouts for Desktops/Servers:Firefox RCE+LPE) [3] https://xkcd.com/2030/ https://xkcd.com/2030/ [4] https://www.zdnet.com/article/microsoft-70-percent-of-all-security-bugs-are-memory-safety-issues/ https://www.zdnet.com/article/microsoft-70-percent-of-all-se...

Jyaif 5y ago

For at least a decade, and in the teams I was in, C"++" written like this would not pass code review precisely because it is incredibly brittle.

tialaramex 5y ago
Uhuh. On cue a C++ programmer arrives to tell us that a true Scotsman wouldn't have introduced this bug... Where can we see "at least a decade" of this code you and your teams wrote?

Stampo00 5y ago

Why don't we have linters that would complain about copying memory without bounds checking?

deleted 5y ago

[deleted]

mleonhard 5y ago

What went wrong - Issue #0: The library was not re-written in a language that prevents undefined behavior (UB).

tialaramex 5y ago
I don't think there are any general purpose programming languages with decent performance which outright "prevent undefined behaviour" in something like NSS. Rust, for example, does not. safe Rust doesn't have undefined behaviour but of course you can (and a large project like this will) use unsafe Rust and then you need the same precautions for that code. This sharply reduces your exposure if you're doing a decent job - and is therefore worthwhile but it is not a silver bullet. Outright preventing undefined behaviour is hard. Java outlaws it, but probably not successfully (I believe it's a bug if your Java VM exhibits undefined behaviour, but you may find that unless it was trivial to exploit it goes in the pile of known bugs and nobody is jumping up and down to fix it). Go explains in its deeper documentation that concurrent Go is unsafe (this is one of the places where Rust is safer, safe Rust is still safe concurrently). Something like WUFFS prevents undefined behaviour and has excellent performance but it has a deliberately limited domain rather than being a general purpose language. Perhaps a language like WUFFS should exist for much of the work NSS does. But Rust does exist, it was created at Mozilla where NSS lives, and NSS wasn't rewritten in Rust so why should we expect it to be rewritten in this hypothetical Wrangling Untrusted Cryptographic Data Safely language?
- mleonhard 5y ago
  > I don't think there are any general purpose programming languages with decent performance which outright "prevent undefined behaviour" in something like NSS. Rust, for example, does not. You know what I meant by "a language that prevents UB". Your comment argues semantics. That's not nice. Please stop. > safe Rust doesn't have undefined behaviour but of course you can (and a large project like this will) use unsafe Rust and then you need the same precautions for that code. How can that can be true? A large Rust project uses unsafe only because its authors don't care enough. Instead of spending the effort to make the safe code fast enough, they resort to using `unsafe`. It's the same reason that people add code without tests or add APIs without docs. A special purpose language for writing cryptography code is a terrible idea. Once we have a language suitable for handling untrusted data, we should make it good enough to use for everything and then use it for everything. I have put some effort into making safe Rust usable for everything. I wrote a safe regex library [0] and a safe async library [1]. I plan to put more effort into these and other libraries. For example, I am working on safe Rust TLS and HTTP libraries. Eventually, safe Rust will be production quality and fast enough for most use-cases. [0] https://crates.io/crates/safe-regex https://crates.io/crates/safe-regex [1] https://crates.io/crates/safina https://crates.io/crates/safina

deleted 5y ago

[deleted]

nielsole 5y ago

> Issue #2 Arbitrary size limits.[...] > A reasonable choice might be 2^24-1 bytes, the largest possible certificate How does one treat untrusted input whose length might exceed available memory? I am working on a patch for a jwks implementation which does not even have upper bounds in the spec. Accepting any valid input until OOMing seems like a suboptimal solution.

duped 5y ago
In a sense, reducing the error case to the physical limitations of a device is a perfectly "optimal" solution
saagarjha 5y ago
Generally, taint tracking.

rurban 5y ago

Now take a deep look at the POSIX C standard, Annex K. The bounds checked extensions. Using these would have definitely avoided the problem. memcpy_s requires the size of dest to be defined. The applause goes to the glibc maintainers, who still think they are above that.

ndesaulniers 5y ago
Doesn't memcpy_s not take into account the sources size though? You could still read past the end of an object with it, right?
- rurban 5y ago
  nope, it checks both. How could you call it secure without checking both sizes? memcpy_s(dest,dmax,src,slen) and my implementation safeclib even at compile-time, similar to glibc's FORTIFY.
loeg 5y ago
Annex K is pretty awful[1]. There are plenty of fine solutions here in hindsight, but glibc adopting Annex K isn’t one of them. NSS has a plethora of build targets, including Windows - which does not implement Annex K either (despite inspiring it). [1]: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1967.htm http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1967.htm
- rurban 5y ago
  Wrong. First it's good, not awful. Awful are only the ones not using it. Second, Windows implements Annex K as only major provider. The others are some minor embedded targets, plus Android Bionic recently. Implementations, such as safeclib, are cross platform. you can use it everywhere. Security and crypto people per se don't use it (incompetence or not invented here), but security aware people, such as on embedded or in the industry.

albntomat0 5y ago

Since this comes up whenever there is a Project Zero article, here is a summary I made in summer 2020 on the distribution of the bugs they find/report: Since this always comes up, here's an overview I made several weeks ago about where Project Zero focuses their efforts: All counts are rough numbers. Project zero posts: Google: 24 Apple: 28 Microsoft: 36 I was curious, so I poked around the project zero bug tracker to try to find ground truth about their bug reporting: https://bugs.chromium.org/p/project-zero/issues/list https://bugs.chromium.org/p/project-zero/issues/list For all issues, including closed: product=Android returns 81 results product=iOS returns 58 vendor=Apple returns 380 vendor=Google returns 145 (bugs in Samsung's Android kernel,etc. are tracked separately) vendor=Linux return 54 To be fair, a huge number of things make this not an even comparison, including the underlying bug rate, different products and downstream Android vendors being tracked separately. Also, # bugs found != which ones they choose to write about.

deleted 5y ago
[deleted]
gausswho 5y ago
I'm not clear. What do these varying counts imply to you?
- nova22033 5y ago
  Because someone will invariably accuse project zero of being hostile to google's competitors.
sangnoir 5y ago
> I made several weeks ago about where Project Zero focuses their efforts Are you sure the numbers track where they put effort? I can think of a couple of confounding factors (including P0 methodologies, and number of low-hanging[1] bugs in target products) 1. Relatively speaking
- albntomat0 5y ago
  I absolutely agree that there are numerous reasons why the numbers are not equal. My sole reason for posting is the "P0 is a hit squad against Apple/Mozzilla/Microsoft" comments that come up whenever their blog posts end up on HN.

nomoreusernames 5y ago

so who wants to tell linus to rewrite everything in rust?

jmull 5y ago

To me, PORT_Memcpy is one problem here. There are two buffers and one size -- the amount of memory to copy. There should be PORT_Memcpy2(pDest, destSize, pSource, numBytesToCopy) (or whatever you want to call it) which at least prompts the programmer to account for the size destination buffer. Then flag all calls to PORT_Memcpy and at least make a dev look at it. (Same for the various similar functions like strcpy, etc.)

twodayslate 5y ago
Of course it would just end up being PORT_Memcpy2(cx->u.buffer, sigLen, sig->data, sigLen);
- jmull 5y ago
  Someone could do that, but the point of having the dest buffer size is to at least give the programmer a chance to try to get it right. I also wonder if a linter could notice that the dest buffer size passed isn’t the actual size of the buffer. (That leads the the next problem in the code, if you look at the definition of that buffer, so that’s good.)
ndesaulniers 5y ago
numBytesToCpy != sourceSize Otherwise that's a potential read out of bounds.

Eduard 5y ago

How realistic is it that this vulnerability can be exploited for $BAD_THINGS? https://www.mozilla.org/en-US/security/advisories/mfsa2021-51/ https://www.mozilla.org/en-US/security/advisories/mfsa2021-5... notes "This vulnerability does NOT impact Mozilla Firefox. However, email clients and PDF viewers that use NSS for signature verification, such as Thunderbird, LibreOffice, Evolution and Evince are believed to be impacted.".

sitkack 5y ago

All ASN1 parsers need to get replaced with safe Rust code, full stop.

jlakjsdlkjfowif 5y ago

This is what happens when you let people write C that don't know what they are doing. Probably a new grad ?

aoetalks 5y ago

When will we switch to memory safe (but reasonably performant) languages like Go/Rust/C#? Performance critical sections could remain in C/C++, but MOST code doesn’t need the kind of performance C/C++ provides. Hopefully C/C++ will go the way of assembly: present in TINY doses.

shp0ngle 5y ago
I don't think you need to explain to Mozilla about Just Rewriting It to Rust
- lmm 5y ago
  Didn't they recently fire the Rust team?
  - steveklabnik 5y ago
    While they did let go the folks who were working on Rust (see the other comments in this thread) they are still members of the Rust foundation, and folks still write code in Rust at Mozilla. For example you have this from last week https://groups.google.com/a/mozilla.org/g/dev-platform/c/dVUHF0JY5G4/m/c9VyDijbAwAJ https://groups.google.com/a/mozilla.org/g/dev-platform/c/dVU...

brundolf 5y ago

Which end-user applications are affected? And what would an attacker have to do to exploit this in the wild?

DikshaMA 5y ago

Packers and Movers Bangalore - Reliable and Verified Household Shifting Service Providers Give Reasonable ###Packers and Movers Charges. Cheap and Best Office Relocation Compare Quotation for Assurance for Local and Domestic House Shifting and Get estimates today to save upto 20%, **Read Customer Reviews - @ https://packersmoversbangalore.in/ https://packersmoversbangalore.in/

sneak 5y ago

You'd think that in the wake of "goto fail;" the organization that pioneered rust would have rewritten their core TLS certificate checking library in rust by now, hacker news tropes notwithstanding.

throwaway5371 5y ago

i love the fact that people can read and write whatever they want wherever they want go and rust make things a bit boring, c makes me feel like a kid of course the price for freedom is security haha