Where's My Fox Security Review
- 1 Introduction
- 2 Resources
- 3 Areas of Interest
This is a review of the Where's my Fox server.
- Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=935725
- Wiki: https://wiki.mozilla.org/Services/WheresMyFox
- Source Code: https://github.com/jrconlin/wmf/
- Development Deployment: http://ec2-54-241-87-238.us-west-1.compute.amazonaws.com/
Areas of Interest
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.
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 ?
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.
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.
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.
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.
There is a comment in the Persona code ("Time to UberFake! THIS IS VERY DANGEROUS!") that I do not understand.
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.
Just a MapBox key.
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?
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?
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.