stripe-archive / safesql

Static analysis tool for Golang that protects against SQL injections
MIT License
564 stars 47 forks source link

Add gorm and sqlx support, make easy add new other ORM #10

Closed sergeylanzman closed 6 years ago

sergeylanzman commented 7 years ago
sergeylanzman commented 6 years ago

@zenazn any updates?

sergeylanzman commented 6 years ago

@brandur-stripe any updates?

brandur-stripe commented 6 years ago

I must admit that this is the first time I've seen this project :/

Thanks a lot for the hard for the patch and sorry that about the lack of responsiveness here.

@carl-stripe Do you have any feelings on how we should proceed on this one? Can we get this project back on life support?

clundquist-stripe commented 6 years ago

It has been a minute since I've done go, but it looks like this branch explodes a little harder when used incorrectly.

This branch:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x1276f4c]

Existing master:

cannot find package "<redacted>" in any of:
error loading packages [<redacted>]: no initial packages were loaded
sergeylanzman commented 6 years ago

@clundquist-stripe Strange We use this fork in our CI half year and it works go1.9.2 ubuntu/mac before we use go1.8

Enabling support for database/sql
Enabling support for github.com/jinzhu/gorm

database driver functions that accept queries:
- func (*database/sql.Conn).ExecContext(ctx context.Context, query string, args ...interface{}) (database/sql.Result, error) (param 1)
- func (*database/sql.Conn).QueryContext(ctx context.Context, query string, args ...interface{}) (*database/sql.Rows, error) (param 1)
- func (*database/sql.Conn).QueryRowContext(ctx context.Context, query string, args ...interface{}) *database/sql.Row (param 1)
- func (*database/sql.Conn).PrepareContext(ctx context.Context, query string) (*database/sql.Stmt, error) (param 1)
- func (*database/sql.DB).PrepareContext(ctx context.Context, query string) (*database/sql.Stmt, error) (param 1)
- func (*database/sql.DB).Prepare(query string) (*database/sql.Stmt, error) (param 0)
- func (*database/sql.DB).ExecContext(ctx context.Context, query string, args ...interface{}) (database/sql.Result, error) (param 1)
- func (*database/sql.DB).Exec(query string, args ...interface{}) (database/sql.Result, error) (param 0)
- func (*database/sql.DB).QueryContext(ctx context.Context, query string, args ...interface{}) (*database/sql.Rows, error) (param 1)
- func (*database/sql.DB).Query(query string, args ...interface{}) (*database/sql.Rows, error) (param 0)
- func (*database/sql.DB).QueryRowContext(ctx context.Context, query string, args ...interface{}) *database/sql.Row (param 1)
- func (*database/sql.DB).QueryRow(query string, args ...interface{}) *database/sql.Row (param 0)
- func (*database/sql.Tx).PrepareContext(ctx context.Context, query string) (*database/sql.Stmt, error) (param 1)
- func (*database/sql.Tx).Prepare(query string) (*database/sql.Stmt, error) (param 0)
- func (*database/sql.Tx).ExecContext(ctx context.Context, query string, args ...interface{}) (database/sql.Result, error) (param 1)
- func (*database/sql.Tx).Exec(query string, args ...interface{}) (database/sql.Result, error) (param 0)
- func (*database/sql.Tx).QueryContext(ctx context.Context, query string, args ...interface{}) (*database/sql.Rows, error) (param 1)
- func (*database/sql.Tx).Query(query string, args ...interface{}) (*database/sql.Rows, error) (param 0)
- func (*database/sql.Tx).QueryRowContext(ctx context.Context, query string, args ...interface{}) *database/sql.Row (param 1)
- func (*database/sql.Tx).QueryRow(query string, args ...interface{}) *database/sql.Row (param 0)
- func (*github.com/jinzhu/gorm.DB).Where(query interface{}, args ...interface{}) *github.com/jinzhu/gorm.DB (param 0)
- func (*github.com/jinzhu/gorm.DB).Or(query interface{}, args ...interface{}) *github.com/jinzhu/gorm.DB (param 0)
- func (*github.com/jinzhu/gorm.DB).Not(query interface{}, args ...interface{}) *github.com/jinzhu/gorm.DB (param 0)
- func (*github.com/jinzhu/gorm.DB).Select(query interface{}, args ...interface{}) *github.com/jinzhu/gorm.DB (param 0)
- func (*github.com/jinzhu/gorm.DB).Group(query string) *github.com/jinzhu/gorm.DB (param 0)
- func (*github.com/jinzhu/gorm.DB).Having(query interface{}, values ...interface{}) *github.com/jinzhu/gorm.DB (param 0)
- func (*github.com/jinzhu/gorm.DB).Joins(query string, args ...interface{}) *github.com/jinzhu/gorm.DB (param 0)
- func (*github.com/jinzhu/gorm.DB).Raw(sql string, values ...interface{}) *github.com/jinzhu/gorm.DB (param 0)
- func (*github.com/jinzhu/gorm.DB).Exec(sql string, values ...interface{}) *github.com/jinzhu/gorm.DB (param 0)
- func (*github.com/jinzhu/gorm.Scope).Raw(sql string) *github.com/jinzhu/gorm.Scope (param 0)

You're safe from SQL injection! Yay \o/

safesql

Maybe a problem with your local machine? Master should be work

clundquist-stripe commented 6 years ago

Oh, I left some context out, let me provide the examples I was testing.

So when the package is missing, the branch segfaults instead of returning an error message.

$ ~/go/src/github.com/stripe/safesql/safesql -v does/not/exist
cannot find package "does/not/exist" in any of:
    /Users/clundquist/go/src/<redacted>/vendor/does/not/exist (vendor tree)
    /usr/local/Cellar/go/1.9.2/libexec/src/does/not/exist (from $GOROOT)
    /Users/clundquist/go/src/does/not/exist (from $GOPATH)
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x1276fcc]

goroutine 1 [running]:
main.getImports(0x0, 0x0)
    /Users/clundquist/go/src/github.com/stripe/safesql/safesql.go:221 +0x6c
main.main()
    /Users/clundquist/go/src/github.com/stripe/safesql/safesql.go:68 +0x3a3

The branch works great though if you provide only valid package names like this.

 ~/go/src/github.com/stripe/safesql/safesql -v $(go list ./... | xargs)
Enabling support for database/sql
database driver functions that accept queries:
- func (*database/sql.Conn).ExecContext(ctx context.Context, query string, args ...interface{}) (database/sql.Result, error) (param 1)
- func (*database/sql.Conn).QueryContext(ctx context.Context, query string, args ...interface{}) (*database/sql.Rows, error) (param 1)
- func (*database/sql.Conn).QueryRowContext(ctx context.Context, query string, args ...interface{}) *database/sql.Row (param 1)
- func (*database/sql.Conn).PrepareContext(ctx context.Context, query string) (*database/sql.Stmt, error) (param 1)
- func (*database/sql.DB).PrepareContext(ctx context.Context, query string) (*database/sql.Stmt, error) (param 1)
- func (*database/sql.DB).Prepare(query string) (*database/sql.Stmt, error) (param 0)
- func (*database/sql.DB).ExecContext(ctx context.Context, query string, args ...interface{}) (database/sql.Result, error) (param 1)
- func (*database/sql.DB).Exec(query string, args ...interface{}) (database/sql.Result, error) (param 0)
- func (*database/sql.DB).QueryContext(ctx context.Context, query string, args ...interface{}) (*database/sql.Rows, error) (param 1)
- func (*database/sql.DB).Query(query string, args ...interface{}) (*database/sql.Rows, error) (param 0)
- func (*database/sql.DB).QueryRowContext(ctx context.Context, query string, args ...interface{}) *database/sql.Row (param 1)
- func (*database/sql.DB).QueryRow(query string, args ...interface{}) *database/sql.Row (param 0)
- func (*database/sql.Tx).PrepareContext(ctx context.Context, query string) (*database/sql.Stmt, error) (param 1)
- func (*database/sql.Tx).Prepare(query string) (*database/sql.Stmt, error) (param 0)
- func (*database/sql.Tx).ExecContext(ctx context.Context, query string, args ...interface{}) (database/sql.Result, error) (param 1)
- func (*database/sql.Tx).Exec(query string, args ...interface{}) (database/sql.Result, error) (param 0)
- func (*database/sql.Tx).QueryContext(ctx context.Context, query string, args ...interface{}) (*database/sql.Rows, error) (param 1)
- func (*database/sql.Tx).Query(query string, args ...interface{}) (*database/sql.Rows, error) (param 0)
- func (*database/sql.Tx).QueryRowContext(ctx context.Context, query string, args ...interface{}) *database/sql.Row (param 1)
- func (*database/sql.Tx).QueryRow(query string, args ...interface{}) *database/sql.Row (param 0)

You're safe from SQL injection! Yay \o/

The binary was built from:

commit 58494a48fa4efecb823637e0377b6eb91908a641 (HEAD -> gettaxi, gettaxi/master)
Merge: 452e37e b1a8e63
Author: Sergey Lanzman <sergeylanz@gettaxi.com>
Date:   Mon Mar 6 13:53:28 2017 +0200

    Merge pull request #1 from gettaxi/add-gorm-support

    Add gorm and sqlx support, make easy add new other ORM

I'm a little unclear on how to enable gorm support, but we don't use it, so that could be why it doesn't show up?

sergeylanzman commented 6 years ago

yes if you do not use gorm(not in imports) in your project it not check gorm https://github.com/stripe/safesql/pull/10/files#diff-58204239ceb9412e3b2b853a83ca5b10R71

clundquist-stripe commented 6 years ago

Cool, have any thoughts why bad package names would break things?

sergeylanzman commented 6 years ago

golang.org/x/tools/go/loader

Package loader loads a complete Go program from source code, parsing and type-checking the initial packages plus their transitive closure of dependencies. The ASTs and the derived facts are retained for later use.

I think It same error on the master branch. You must provide right package name

clundquist-stripe commented 6 years ago

On stripe/master we see:

$ safesql -v does/not/exist
cannot find package "does/not/exist" in any of:
    /Users/clundquist/go/src/git.corp.stripe.com/apiori/gopiori/vendor/does/not/exist (vendor tree)
    /usr/local/Cellar/go/1.9.2/libexec/src/does/not/exist (from $GOROOT)
    /Users/clundquist/go/src/does/not/exist (from $GOPATH)
error loading packages [does/not/exist]: couldn't load packages due to errors: does/not/exist

Which is a little better than segfaulting a null pointer, don't you think?

sergeylanzman commented 6 years ago

My bad. Fixed https://github.com/stripe/safesql/pull/10/commits/a7e6848e8f2121c48c095a012370644ef5f810b9