Golang Security Checker

(github.com)

104 points | by ngaut 989 days ago

6 comments

  • pabs3 989 days ago
  • gizdan 989 days ago
    Highly recommend running the whole golangci-lint suite on your Go projects. They're a bunch of great linters being run, and gosec is included.
  • Animats 989 days ago
    It's a linter, not a security model verifier, a fuzzer, or a taint checker. It just looks for a short list of known dumb things to do. Useful, but won't catch anything non-obvious.
    • sudhirj 989 days ago
      Most security errors are the kind that that the tools catches, like unsanitized SQL or unescaped HTML. Those targeted by nation states or running high value targets probably need third party sec audits, but this is a perfectly good addition to a CI or pre commit hook.
      • lima 988 days ago
        > Most security errors are the kind that that the tools catches

        There's plenty of common logic errors that static analysis can't catch.

        They may not be readily exploitable by automated scanners, but it doesn't take a nation state.

  • voidnullnil 988 days ago
    They should add a check for comparing an error value to a string [1]. I've seen lots of mission critical software written in Go now that does this. Just today I came across a bunch of if string.Contains(err.Error(), "problem X") {...}.

    1. https://www.lainchan.org/%CE%BB/src/1612397614615.jpg https://www.lainchan.org/%CE%BB/src/1612397701473.jpg https://www.lainchan.org/%CE%BB/src/1612398029019.jpg

  • KingOfCoders 989 days ago
    Is there something like this for Rust?
    • pcwalton 989 days ago
      A combination of clippy [1] and cargo-audit [2] covers several of these issues, though not all (I'm not aware of a tool to check for uses of bad ciphers in Rust for example).

      [1]: https://github.com/rust-lang/rust-clippy

      [2]: https://github.com/rustsec/rustsec

      • tialaramex 988 days ago
        A reasonable strategy for the ciphers would be to avoid implementing non-recommended ciphers altogether. IIRC Go doesn't even provide some of the "obvious" ciphers modes at all in their standard library, because you shouldn't use them. When you're doing exercises as a student this is momentarily annoying - why can't I have this textbook mode? But no production code should use this, so, implementing it just tempts fate and I endorse their approach.

        I notice that rustls for example does not implement DES. So, while a Go program that wants to do DES TLS should get flagged by this tool, a Rust program that wants do DES TLS has to go find a less popular TLS library, which ought to serve as a reminder that this is a bad idea.

        Of course the flip side of this coin is that sure enough there's a Rust crate that just implements DES. If you really want to do DES even in Go this lint tool can't prevent you from renaming it BobsNiceCipher and using it anyway... at some level you need engineering teams who'll say "No".

    • pabs3 989 days ago
    • decodebytes 989 days ago
      grep for unsafe
      • hnlmorg 989 days ago
        Take a look at the rules this tool checks for and then consider that these are only a subset of rules a mature vulnerability checker would test for. Next to none of those conditions would be caught by "grep[ing] for unsafe".

        The problem with software engineering is there are a thousand ways you can accidentally do something wrong when writing and deploying software in a secure environment.

        • tialaramex 988 days ago
          This tool checks 30 things. One of those thirty is literally "G103: Audit the use of unsafe block"

          Is it a coincidence? Nope, sure enough Go's "unsafe" is closely related to the unsafe concept in Rust, it allows you to defeat type safety promises in Go. Go's promises are much weaker than Rust's but it surely does make sense to audit the use of these features and the same goes for Rust's "unsafe" keyword.

          OK. But the other 29 aren't related right? Not so fast. "G601: Implicit memory aliasing of items from a range statement" is in fact about memory aliasing. You can't introduce such aliases in Rust... from safe code. But your unsafe code can introduce an alias that shouldn't exist and thereby introduce incorrect behaviour.

          Now, the Go lint for G601 has a lot of false positives. Practical experience tells us that this causes two problems, and in many cases they get out of control. One is, some people begin causing false positives without knowing it, their code is fine but is always flagged by this check, making it a nuisance. The second is, because it's a nuisance, it becomes trivial for developers to ignore it. "Just another false positive" and so it ceases to have practical value.

          There are a whole bunch of things here that are orthogonal to "grep for unsafe" and should be in a security-focused linter for Rust, but in fact two of these thirty rules are just "grep for unsafe" in Rust.

          • hnlmorg 988 days ago
            I really can't tell if you're intent was to debunk my comment (the tone reads that way but tone can be misinterpreted) however your conclusion ends up making the same point I raised.
        • decodebytes 988 days ago
          Not sure, if you're aware. I was replying to the comment asking for the same tool, but for rust.
          • hnlmorg 988 days ago
            Yes I was aware :)
  • heldsteel7 989 days ago
    Awesome! Thanks for sharing.