1. 13

  2. 6

    (Some) people keep saying that you just need to sanitize strings and that string manipulation and SQL strings as an interface are not broken. ActiveRecord and github are both reasonably old in the game by now, and as we can see it’s easy even for experienced organizations and/or battle-tested frameworks to miss one sanitization step and boom, another injection.

    With a sane interface this exploit wouldn’t have been possible.

    1. 7

      Right. Sanitizing stuff in general should be a job for the producers, not the consumers of an abstraction. That is, the producer of an abstraction must make sure that it can only be used in sensible ways, and otherwise produce error messages in a timely fashion.

      On the other hand, some people unjustifiedly jump from “SQL strings are a bad interface” (which is true because of the “strings” part) to “SQL is a bad interface”. Hey, you could be manipulating SQL syntax trees (with macros) or even SQL itself as a semantic object (with types).

      1. 1

        Agree 100%.

      2. 3

        Right, but SQL makes that harder than it would be otherwise (because it mixes data and code).

        ActiveRecord provides lots of obvious ways to do things right, but it’s still possible to reach into the underlying connection and provide a string.

        Given how far off the beaten track you have to be to hit an issue like this, I’d say it’s closer to a vulnerability derived from using unsafe in eg rust/go (that is, there should have been alarm bells when the code was reviewed).

        1. 3

          No. As pyon says, SQL doesn’t necessarily mix data and vode in strings, database interfaces do. But even when they don’t, ActiveRecord makes no effort to use e.g. prepared statements. It relies entirely on its sanitization. Even when you use the very cool query DSL in AR, in the end it makes a string.

          (at least back when i was using RoR 3)