Where's My Fox Security Review

From MozillaWiki
Jump to: navigation, search

Introduction

This is a review of the Where's my Fox server.

Resources

Areas of Interest

SQL Injection

The application uses the native Go Postgres driver with prepared/bound statements (almost) everywhere.

There is one exception at https://github.com/jrconlin/wmf/blob/master/src/mozilla.org/wmf/storage/storage.go#L441 where due to limitations of the driver? an integer value has to be sprintf'd into a statement. But I think this is fine because it is done as a native integer value.

Other Injection

HIGH RISK - There is a change of JSON injection in the Register() handler at https://github.com/jrconlin/wmf/blob/master/src/mozilla.org/wmf/handlers.go#L421 .. specially since the deviceid is blindly copied from the POST data. This could be abused by injecting HTML into JSON output, which is then interpreted by some browsers as content. (I think the Register handler also forgets to set the content-type, which make this attack even more probably)

It is generally better to generate JSON using json.Marshal()

Direct File Access

To serve its static content, the app does a ioutil.ReadFile("." + req.URL.Path) ... I see there is a check for ".." above that but it still gives me the creeps. Is there no better way to do this? Also are we totally sure that filtering on ".." is enough? Maybe that code should only work in development mode while production mode uses a front-end proxy to service /static ?

Process Execution

Nope.

Templates

HIGH RISK - The application uses the wrong template engine. GO provides two: text/template and html/template. The latter makes sure that template variables are properly escaped in a HTML context. But the app uses the former: text/template.

To resolve, the app needs to switch to html/template.

Authorization

HIGH RISK - The application uses a SHA256 encoded email address in a cookie to authenticate the user. This can easily be spoofed by the user, which will lead to access to a users devices.

To resolve, it is suggested to use a real session store and set the cookie to a completely random session identifier that can then be looked up in server-side session database. This indirection prevents people from constructing cookies.

Headers

I don't think all Handlers correctly set their content-type. There seem to be a bunch of API endpoints that do not do it and I wonder if that means they default to text/plain or text/html.

Cookies

In https://github.com/jrconlin/wmf/blob/master/src/mozilla.org/wmf/utils.go#L85 the generated Cookie will need to also have the Secure and HTTP flags set to true.


Persona Integration

There is a comment in the Persona code ("Time to UberFake! THIS IS VERY DANGEROUS!") that I do not understand.

https://github.com/jrconlin/wmf/blob/master/src/mozilla.org/wmf/handlers.go#L73

This might be just code for testing. If so then it should probably be removed?

API Input Validation

HIGH RISK - Input validation should be more structural and part of every handler

Functions like getDevFromUrl() to extract a device identifier from the url do not make an effort to validate the identifier. If we know these identifiers are hex hashes or uuids then I think that code can be stricter by matching them against a regular expression.

I think ideally every handler spends a little time right at the beginning, before it even opens a connection to the database, to make sure that what it receives in query parameters or post body is valid.

Another example, in the Register() handler it is possible provide a device id in the POSTed JSON that has any form. Similarly the accepts array could be validated to make sure it only contains valid options, no duplicates and a reasonable length.

https://github.com/jrconlin/wmf/blob/master/src/mozilla.org/wmf/handlers.go#L319

API Keys

Just a MapBox key.

API Abuse

Resource exhaustion, the usual. Not sure if we should be worried about it in the app itself. May be better for a front-end proxy?

Internals Exposure

Are /metrics and /status supposed to be public? I don't think they expose anything critical but still .. is this info we want others to capture?

Frontend

I see some inline javascript in the frontend. I understand this is not the final frontend, but I still want to raise this because it would be in the way of applying a proper CSP policy to this application. (Which means no inline script of style)

Deployment

This app is supposed to deal with a potentially large number of websocket connections. AFAIK we do not have a front-end proxy that can properly deal with this. Does that mean this app will be exposed directly to the public internet? If so, will the app have to implement specific things that we normally do in Zeus or HAProxy? (Maybe i am wrong about this)

The development deployment does not send out basic security headers. We would like the app to send out XFO, HSTS and CSP.

Ideally we do a Staging Deployment under HTTPS with all required security parameters enabled. Then we can a final assessment of things like HSTS, cookies, redirect to SSL.

Retrieved from "https://wiki.mozilla.org/index.php?title=Where%27s_My_Fox_Security_Review&oldid=965631"