Settings

Theme

Fuzzing Go APIs for SQL Injection

blog.fuzzbuzz.io

64 points by andrei 3 years ago · 22 comments

Reader

spullara 3 years ago

People are still constructing SQL statements using user provided data? Have they never used prepared statements before?

  • oefrha 3 years ago

    Looks more like a contrived example to me:

      h.db.Exec(fmt.Sprintf("insert into users(name, email, phone_number) values ('%s', '%s', '%s');",
           request.Name, request.Email, request.PhoneNumber))
    
    Any database driver including database/sql will support

      h.db.Exec("insert into users(name, email, phone_number) values (?, ?, ?);",
           request.Name, request.Email, request.PhoneNumber)
    
    which is shorter and more natural. They’re throwing in a fmt.Sprintf in there for no reason other than forcing a tired old SQL injection.

    Now, shitty HTML templating causing injection with unsanitized user input is a lot more realistic, since the golang templating story isn’t great.

    Edit: “templating” -> “HTML templating”.

    • jrockway 3 years ago

      It doesn't really matter that it's contrived. It's hard to show the value of something in a large system, because 90% of the blog post will be understanding that large system. So you have to pick something that is immediately obviously wrong, and let the reader make the jump to how this would actually be useful in the context of their own larger system.

      Even then, the approach of "just do it right the first time" is fallible. People have varying degrees of experience, and people have brain farts and type the wrong thing instead of the write thing. Your job as the technical lead is to have some sort of process in place to catch these things before they become security disasters. "Sorry, our junior engineer didn't know about prepared statements," is not something you want to tell your customers whose data was exfiltrated. The current industry standard here would be "cross your fingers and hope for the best", and that's why everyone knows your social security number and have opened 6 credit cards in your name. Fuzzing is another layer of sanity checking, on top of static analysis (where I think this issue should be caught), code reviews, and just typing in the correct code in the first place.

      At the end of the day, this just another example of how you could use fuzzing to detect problems that other things missed. That's valuable, as even with 100% code coverage and turning on all the lint checks, software still has bugs. Here's a tool that can help reduce the bug count.

    • evmunro 3 years ago

      You're right, it's likely that almost nobody is using `fmt.Sprintf` to build SQL queries in production.

      Templating and `fmt.Sprintf` are essentially the same thing in this context - `Sprintf` just gets the point across in fewer lines of code, and allows people to come up with realistic scenarios themselves.

      • oefrha 3 years ago

        I’m talking about HTML templating, not templating SQL statements.

  • andreiOP 3 years ago

    It's much more common than you may think - especially at larger organizations where engineers go "off-script" frequently.

    That being said, we wanted to highlight an example of how fuzzing can be applied to a typical (albeit, toy) API to find logic bugs, and figured SQL Injection would be something that resonated with most (all?) developers.

    • DylanSp 3 years ago

      It's fairly obvious that it's a contrived example, though, which means it's not much of a motivating example for fuzz testing. I'd think it'd help to have a non-obvious bug that's triggered by an unusual set of inputs; that would show the value of fuzz testing mich better. (Admittedly, this is easier said than done)

      EDIT: I tried using fuzz testing to find the famous issue with integer overflows in binary search [1], but even when restricting the relevant type to uint8, a couple of minutes of fuzzing when running on gitpod.io didn't detect an issue. Repo is https://github.com/DylanSp/fuzzing-for-binary-search-overflo... if anyone wants to play around with it and see if they can get fuzzing to detect a problem. (Go doesn't panic on overflows; a different approach to creating the slice to search might reveal a logic error)

      [1] https://ai.googleblog.com/2006/06/extra-extra-read-all-about...

  • kkirsche 3 years ago

    The problem I run into at work is developers learn ORMs in many situations before they learn SQL itself. As a result, many are used to ORMs which can do this for them via parameterization or they simply were never exposed to them. Whether it’s good or bad, right or wrong, we’ve found that ensuring certain concepts are shared across all developers via a team onboarding training trumps the inconvenience when they already know these.

  • KingOfCoders 3 years ago

    Yes, it feels dated since JDBC brought prepared statements into the mainstream literally decades ago. Though I have seen this still happening from marketing hiring external marketing agencies to write some small apps for them, e.g. a web game for grabbing email addresses. Hopefully they don't have access to your main database (though I have seen this in startups too, because fast,fast,fast).

    • pjmlp 3 years ago

      ODBC, ADO and Perl DBI did it even earlier.

      • KingOfCoders 3 years ago

        Agree, my first database driven website was with DBI (also writing the German Strato web and mail plattform) in the 90s with Hughes mSQL and Delphi ODBC, my feeling though was JDBC brought database driven websites to the mainstream.

  • sa46 3 years ago

    Prepared statements (in Postgres) don't work for:

    - Dynamically generated queries, like a user specifying a query predicate via a UI.

    - Dynamically selecting an identifier, like `SELECT * FROM $1` or `SELECT $1, count(*) FROM foo GROUP BY $1`.

    You can use cleverness with types to parse user-provided data into a struct that emits sanitized SQL, but under the hood, there are only strings.

    • spullara 3 years ago

      Then you can't allow users to provide them. They must be constructed with constraints.

  • pjmlp 3 years ago
  • henvic 3 years ago

    I'm afraid that even on the Go ecosystem you'll find people relying on ORMs that doesn't even use server-side prepared statements, but escapes queries ¯\_(ツ)_/¯

    bun, I'm looking at you...

    * IMHO there is not even any good reason to use an ORM whatsoever (unless, perhaps, you're building something very very very very very specific that needs to deal with very very very generic data).

andreiOP 3 years ago

A lot of folks we talk to think fuzzing is only useful for finding memory leaks in C++ programs, so we wanted to show how adding a single fuzz test to your API can find SQL injection and other logic bugs.

Would love to hear others' experience with Go fuzzing now that it's been out for a few months.

KingOfCoders 3 years ago

Is there a open source fuzzing framework for Go?

Keyboard Shortcuts

j
Next item
k
Previous item
o / Enter
Open selected item
?
Show this help
Esc
Close modal / clear selection