Skip to content

Add link to github.com/bored-engineer/safe-squirrel to README? #387

Open
@bored-engineer

Description

The README indicates you may be open to linking forks/related projects:

Squirrel is "complete". Bug fixes will still be merged (slowly). Bug reports are welcome, but I will not necessarily respond to them. If another fork (or substantially similar project) actively improves on what Squirrel does, let me know and I may link to it here.

I'd like to propose/discuss adding a link to my fork bored-engineer/safe-squirrel which enforces more secure/safe usage via the Golang type system to prevent SQL injection...

The squirrel package already encourages the use of parameterized queries (aka placeholders) to reduce the risk of SQL injection, ex:

username := "bored-engineer" // untrusted input

// "SELECT * FROM users WHERE github = ?"
sq.Select("*").From("users").Where(sq.Eq{"github": username}).ToSql()

However, not all methods/parameters in squirrel are safe/protected against SQL injection, ex:

provider := "is_superadmin=true OR github" // untrusted input
username := "uh oh" // untrusted input

// "SELECT * FROM users WHERE is_superadmin=true OR github = ?"
sq.Select("*").From("users").Where(sq.Eq{provider: username}).ToSql()

While this is a contrived example, SQL injection vulnerabilities have been found in real-world applications/services that use squirrel due to incorrect usage of these APIs by developers.

My fork aims to systemically prevent these SQL injection vulnerabilities in squirrel at compile-time with minimal/no refactoring. By taking advantage of the Golang type system/compiler, it is possible to create a function that will only accept a const string at compile-time, ex:

type safeString string

func refuseDynamicStrings(foo safeString) {
    println(foo)
}

When this function is invoked Golang will automatically cast const strings to the private (otherwise inaccessible) safeString type:

pkg.refuseDynamicStrings("this is an implicit const string")
const foo = "this is an explicit const string"
pkg.refuseDynamicStrings(foo)

However, if we try to pass a dynamic string (such as one generated from user input), it will fail to build/compile:

var bar = fmt.Sprintf("this is a %s string", "dynamic")
pkg.refuseDynamicStrings(bar) // cannot use bar (variable of type string) as safeString value in argument to refuseDynamicStrings

My fork takes advantage of this "feature" to enforce that all parameters passed to squirrel are const strings at compile-time. APIs that are already secure due to their use of parameterized queries like sq.Expr("foo = ?", untrustedStringVar) or sq.Eq{"column": untrustedStringVar} continue to work as-is, accepting dynamic values on the relevant types.

I would have just proposed these changes as a PR however there are some (minor) breaking changes in my fork as called out in the caveats section of the README which makes this better suited to live as a fork. The full diff can be found here: master...bored-engineer:safe-squirrel:master

Do you think this fork adds enough value to be linked from the main squirrel project's README?

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions