
Go Language – Web Application Secure Coding Practices - rbanffy
https://checkmarx.gitbooks.io/go-scp/
======
buro9
When I see anything that touches web coding practices for Go I always look up
an area that I know best: sanitization.

I wrote [https://github.com/microcosm-
cc/bluemonday](https://github.com/microcosm-cc/bluemonday) which is a pure Go
HTML sanitizer inspired by [https://github.com/owasp/java-html-
sanitizer](https://github.com/owasp/java-html-sanitizer) .

The key things to understand about HTML sanitizers:

* They must be whitelist based

* They must be aware of context

* You must sanitize _ALL_ user input even if you don't think you're going to render it on a web page.

The book linked to in the article does not seem to understand any of the
above.

The section on sanitization has the equivalent of "string replace" as the
primary recommendation. Elsewhere in the XSS section a focus is on escaping
content before it is rendered.

Sanitization needs to know not to run on <pre> blocks, and to escape HTML
entities automatically, and to understand what links are safe and which are
not.

XSS can be really interesting and quite targeted. It can be that a user-agent
contains the XSS, because the target may not be the person reading the page
but the admin looking at a web page of their web server logs through an
analytics program on the same domain.

The bluemonday package I wrote can deal with all of these things but that
isn't the point, the point is that this is an area I know and the book falls
way short of a decent standard for creating a secure and safe web application.
And if it falls short in this area (the first 2 chapters) then I would assume
that it falls short in all areas.

~~~
osteele
> You must sanitize ALL user input even if you don't think you're going to
> render it on a web page.

I'm not able to make sense of this. Sanitize it for what context? SQL? JSON?
HTML? Inclusion as a command-line argument? All of these, and hope that
sanitizing it for one context doesn't un-sanitize it for others?

~~~
adamdoupe
Context here means the context of the output page.

Usually this means the HTML context. Different sanitization is needed
depending on _where_ in the HTML document the input is used.

For instance, if the input is used in between HTML tags (let's say $foo is
user input in this PHP example):

    
    
        ... <body><?php echo $foo ?></body>
    

Here, the input that you need to transition to JavaScript execution is a <
character (among other things): <script>alert(1)</script>.

Therefore, to correctly sanitize this, you would call the PHP `htmlentities`
function:

    
    
        ... <body><?php echo htmlentities($foo) ?></body>
    

Now, this XSS vulnerability is fixed.

What if foo is used in a different context?

    
    
        ... <body><a href='<?php echo htmlentities($foo) ?>'>...
    

Here, what we need to transition the HTML parser to executing JavaScript is a
' character, and this can be exploited by the following input (in between the
double quotes): "' onclick='alert(1)"

The key problem is that `htmlentities` is not valid sanitization in the
context of an HTML attribute value. In this example, you need to use
`urlencode`

    
    
        ... <body><a href='<?php echo urlencode($foo) ?>'>...
    

The general idea also applies to CSS, JSON, and JavaScript. SQL is a different
vulnerability class (SQL injection).

I highly recommend the following research paper from 2011 that discusses the
context-sensitivity of JavaScript in depth:
[http://www.comp.nus.edu.sg/~prateeks/papers/scriptgard-
ccs11...](http://www.comp.nus.edu.sg/~prateeks/papers/scriptgard-ccs11.pdf)

In my mind, the context-sensitivity of XSS is one of the key reasons why it is
so prevalent.

------
Twirrim
Last time this was discussed:
[https://news.ycombinator.com/item?id=14192383](https://news.ycombinator.com/item?id=14192383)

Still not convinced about the usefulness of the source, given the criticisms
raised here and there.

~~~
MajesticHobo
This guide still has some issues. It's missing common classes of web app vulns
I've seen in Go code (e.g. CSRF, SSRF) and has some weird advice here and
there (scan uploaded files with AV? Really?)

~~~
wbl
Fail to scan with AV and you might be an unwitting malware distributor.

~~~
MajesticHobo
If you allow binary uploads, you're going to be a malware distributor whether
you scan or not. AV just introduces complexity and attack surface and doesn't
really belong in a guide about Golang secure coding practices.

------
rgbrenner
Under Validation and Storage > Storing password securely: the theory

That code -- using sha256 to hash a password+salt, and store it in a db --
should not be there. Someone WILL copy paste it.

Don't make it easy for people to do something stupid.

~~~
m_sahaf
To be fair to the author, the paragraph immediately following that code
snippet says:

"However, this approach has several flaws and should not be used. It is given
here only to illustrate the theory with a practical example. The next section
explains how to correctly salt passwords in real life."

and goes on to use bcrypt.

~~~
jszymborski
The fear is that people won't read that or just use the SHA256 example because
it's before the bcrypt one, or maybe because they find it runs faster.

IMHO, I think examples of dangerous code should implement one of the following
mindless-script-kiddie-consultant defenses (in order of most sane to least
sane):

1\. use screenshots of the snippet

2\. use pseudo-code

3\. put in deliberate syntax mistakes

4\. have a javascript alert on right-click or ctrl/cmd+c with the warning
about it being unsafe (a.k.a. going full 90s)

~~~
ams6110
Yes the problem with "unsafe" examples is that people _will_ use them --
perhaps thinking, "this is a POC, we'll come back later and do this properly"
\-- only it gets forgotten and ends up in production.

------
Clex
Go has also an amazing static-analysis tool, vet, that is not mentioned in the
article: [https://golang.org/cmd/vet/](https://golang.org/cmd/vet/)

It can find printf-format errors, invalid shifts, unreachable code, etc.

Even though go vet is very helpful, it is sometimes scary that the compiler
allows to build such incorrect code. For instance:
[https://play.golang.org/p/2AVHUt5Wcf](https://play.golang.org/p/2AVHUt5Wcf)

~~~
Vendan
I'm not sure how `fmt.Printf("%s\n", i) // invalid type` is really that
"scary". Expecting the compiler to introspect that Printf's `...interface{}`
argument's first value is incorrect, even though `i` does fulfill the stated
parameter's type, is a FAR more scary thought.

~~~
_ph_
The important thing here is, that fmt.Printf is just a function which takes a
string and a variable amount of interface{} parameters. There is nothing in
the language spec which creates a type correlation. the "%s" denotes a string
parameter to print is solely some inner working of the Printf function. Yes,
you can special case fmt.Printf, as some compilers do, but given the standard
go vet tool, I rather think it belongs there than into the compiler, which,
following the Go philosophy, exactly understands the Go language and precisely
compiles that.

~~~
Vendan
That's exactly my point. Special casing the compilation of 1 specific set of
functions is a scary concept, esp. as the tool that can check the types is
already in the go toolchain, AND there are plenty of edge cases that can't be
caught anyways, so it can create a false sense of security.

~~~
_ph_
Ah, had not quite caught your point as you had phrased it (and that by the
time I answered your post was difficult to read for me, as for some downvotes
it was printed in a very light font). Then we fully agree.

------
ShabbosGoy
Overall I thought it was a good read. Does anyone know if using bindvars is
foolproof against SQL injection? I'm currently using the sqlx library because
some of the ORMs available for Go are rather clunky to deal with.

------
hendry
I once made a tragic security error with

    
    
         http.Handle("/o/", http.StripPrefix("/o/", http.FileServer(http.Dir("/"))))
    

It's a bit too easy to expose to much of the FS aka "Directory traversal
attack". My advice, always deploy into a container.

------
jalfresi
Theres a whole section on stored procedures - it was my understanding that the
database/sql package in the sodlib didn't support stored procedures (something
to do with multiple rows or output var support)

~~~
camus2
The database/sql package is just bad anyway. The idea is sound but the
execution is catastrophic and not many thought went into the design of this
package. This is especially true with Postgresql where you are kind of forced
to use db.Query instead of db.Exec on inserts or updates. Go std lib have a
lot of issues like these. On the top of my head the log package and the flags
package are pretty useless in real world apps.

------
in9
is there an equivalent of this book for python?

~~~
johnpython
and Ruby as well

------
Dominator
People who are not happy with the current content may contribute to make it
better. This is an open source and collaborative work. There is obviously room
for improvement. But instead of spitting in the soup, provide merge requests
or issues. This book is shared for free. If everybody is giving a little of
its time to make it better, then we may get a really usefull and informative
book. It's a huge work to write a book.

See here how to contribute : [https://checkmarx.gitbooks.io/go-
scp/content/howto-contribut...](https://checkmarx.gitbooks.io/go-
scp/content/howto-contribute.html)

------
homakov
Don't read it, _always_ start with using a good framework. It will do
everything for you

~~~
solarengineer
Hi homakov, given your experience with security, could you please expand on
_why_ we should not this particular guide? I do understand why we should start
with using a good framework, and I have so far felt that reading guides also
help us understand things to be aware of.

~~~
homakov
Because just reading a book or tutorial won't give you enough info to be
comfortable with all terms there. For hobby or if you have spare time - yes go
ahead, for practical business needs - waste of time. Good framework handles
most of them better, and if it doesn't, it's not good one. So step 1 is always
framework. Optional 2 is tutorials.

