Closed Bug 764450 Opened 12 years ago Closed 12 years ago

Perform Security Review for Mozilla Reps Portal 0.3

Categories

(mozilla.org :: Security Assurance: Review Request, task)

Product:
mozilla.org
Component:
Security Assurance: Review Request
Platform:
x86
macOS
Type:
task
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
VERIFIED FIXED

People

(Reporter: bensternthal, Assigned: mfuller)

References

Details

(Whiteboard: [pending secreview][start yyyy-mm-dd][target yyyy-mm-dd]) 

We would like a security review of the 0.3 release of the Mozilla Reps Portal. Our current production date is July 23 and we think the sec review can happen between 7/9 - 7/20. This is a substantial release and the sec review is a launch dependency.


==================================================

Who is/are the point of contact(s) for this review?

Product owner - Pierros Papadeas
Developers - Giorgos Logiotatidis
TPM - Benjamin Sternthal
    
Please provide a short description of the feature / application (e.g. problem solved, use cases, etc.):

This is a sec review of the 0.3 release of the Mozilla Reps website. This is primarily an enhancement release and adds event scheduling and some API interaction with Mozillians. You can see the current scope of the 0.3 release here:

https://wiki.mozilla.org/Websites/ReMo_Mozilla_Reps/Open-Bugs
    
Please provide links to additional information (e.g. feature page, wiki) if available and not yet included in feature description:

The project wiki contains everything about this project and we try to make sure this is truth. Both the list of features and current schedule can be seen here. The weekly status report is also a handy overview of our current dates:

https://wiki.mozilla.org/Websites/ReMo_Mozilla_Reps
    
Does this request block another bug? If so, please indicate the bug number This review will be scheduled amongst other requested reviews. What is the urgency or needed completion date of this review?

This request blocks our launch of 0.3.

Please answer the following few questions: (Note: If you are asked to describe anything, 1-2 sentences shall suffice.)

Does this feature or code change affect Firefox, Thunderbird or any product or service the Mozilla ships to end users?

This project only affects the mozilla reps website.

Are there any portions of the project that interact with 3rd party services?

We use a third party for the interactive map: https://reps.mozilla.org/people/ this has already been security reviewed in a previous release


Will your application/service collect user data? If so, please describe 

Yes, however this functionality is already in the app and live. I do not think there are major changes to this included in the 0.3 release.

If you feel something is missing here or you would like to provide other kind of feedback, feel free to do so here (no limits on size):

Desired Date of review (if known from https://mail.mozilla.com/home/ckoenig@mozilla.com/Security%20Review.html) and whom to invite.

We have the sec review scheduled for 7/9 - 7/20. We would like to request Yvan as he has familiarity with the app and is a pleasure to work with
yvan is PTO so this may have to sit for a while if it is assigned to him
Whiteboard: [pending secreview][triage needed 2012.06.13]
It's not needed until 7/9 - 7/20.
This isn't needed until 7/9 but is it ready now? If so, we can begin now.
No not ready yet. I was filling this in advance.
Depends on: 732378
Assignee: nobody → yboily
Whiteboard: [pending secreview][triage needed 2012.06.13] → [pending secreview][start yyyy-mm-dd][target yyyy-mm-dd]
:bensternthal - just checking to see if this is ready since the target was 7/9 - 7/20. Let me know and I can start
Thanks!
Matt
Matt...the date on this did slip (you can always check the wiki for the latest schedule). I should have updated this bug... so apologies on that.

We should have this code deployed to stage monday so dates for sec review is 

7/23 - 7/27
No problem, thanks for the update! I'll begin on Monday unless you run into any issues, in which case just comment on here and I'll postpone it.

Matt
Matt -> quick update on this. We moved the stage deployment date to tomorrow (7/24). I'll update this bug when the stage request is filed so you know when to start.
It looks like the push to stage was made so I'm going to start on this - let me know if anything changes

Matt
Hi Ben,

I'm mostly finished the security review for this site, and just wanted to let you know about a few things I found.

- The admin page (https://reps.allizom.org/admin/) is publicly accessible. Normally, if that page isn't needed, it's deactivated/restricted to internal IPs, etc. However, if it is used/needed, it should have a lockout feature that prevents bruteforce attacks which it currently doesn't.

- In Chrome, I receive a mixed content warning when viewing the homepage, which I determined was because the Blog Feed is loaded via an http://ajax.googleapis.com link. Google does offer HTTPS for those links, so changing it from http to https should solve that problem.

- Strict Transport Security is not set for the site. Normally, we recommend that any site using HTTPS also set STS. You can read more about that here: https://developer.mozilla.org/en/Security/HTTP_Strict_Transport_Security

- The APIs which are used to retrieve data (ex: https://reps.allizom.org/api/v1/rep/) are not protected via any kind of token that would prevent direct access and content scraping of all users' emails, full names, locations, etc. Although this isn't really a security issue, I just wanted to make sure that the privacy policy of the site makes users aware they will be releasing all of that data to the public.

Other than that, most things look good and I should be finishing this up shortly. Let me know about the above issues and we'll go from there.

Thanks!
Matt
Also, another issue:

- The description field content is not escaped properly. This allows for XSS / code to be injected into the page. View the URL here: https://reps.allizom.org/u/mfuller/ to see the alert.
Depends on: 777656
Hi Matt,

thanks for your review. Inline comments follow

> 
> - The admin page (https://reps.allizom.org/admin/) is publicly accessible.
> Normally, if that page isn't needed, it's deactivated/restricted to internal
> IPs, etc. However, if it is used/needed, it should have a lockout feature
> that prevents bruteforce attacks which it currently doesn't.

There is bug 754789 about /admin being behind vpn for production. I guess we should add /admin behind vpn or apache auth or even leave it as is, since it's just demo data? 

Jason, comments on this please?


> 
> - In Chrome, I receive a mixed content warning when viewing the homepage,
> which I determined was because the Blog Feed is loaded via an
> http://ajax.googleapis.com link. Google does offer HTTPS for those links, so
> changing it from http to https should solve that problem.

Fixed that on dev with bug 777656 . Also moved a link html5.js to https (when using IE).

> 
> - Strict Transport Security is not set for the site. Normally, we recommend
> that any site using HTTPS also set STS. You can read more about that here:
> https://developer.mozilla.org/en/Security/HTTP_Strict_Transport_Security

Jason, comments on this please?

> 
> - The APIs which are used to retrieve data (ex:
> https://reps.allizom.org/api/v1/rep/) are not protected via any kind of
> token that would prevent direct access and content scraping of all users'
> emails, full names, locations, etc. Although this isn't really a security
> issue, I just wanted to make sure that the privacy policy of the site makes
> users aware they will be releasing all of that data to the public.

Thanks for the heads up. APIs are publicly accessible to everyone, but not everyone gets the same information. For example email is not available to logged out users, but all other data is, since we have them public on the website anyway. Currently we use browser session for auth, maybe we also support API keys in the future as program specs change.


Thanks!
Depends on: 777667
(In reply to Matt Fuller :mfuller from comment #11)
> Also, another issue:
> 
> - The description field content is not escaped properly. This allows for XSS
> / code to be injected into the page. View the URL here:
> https://reps.allizom.org/u/mfuller/ to see the alert.

Fixed that bug 777667.

Thanks! ;)
Ben,

I'm done with bug fixing. Changes are in reps-dev but I also created a new tag (v0.3.1) so we can push to stage again. 

Thanks!
Hi, I've re-reviewed and comments are below. Overall, looks good.

- As long as the admin page will be blocked/restricted on the production instance, I see no issues leaving it on stage (unless authentication information is the same/similar).

- Mixed content issues are resolved.

- XSS issues resolved.

- For the API, that is fine, I just wanted to make sure you/users were aware of what info is shared/vs not shared with logged in vs logged out users.

- For STS, we highly recommend that it is enabled for all sites using https. If it's something that can't be implemented in time for release, I can check with my team to make sure we can go ahead without it for now, but I would try to get it in.

At this point, it looks like everything is good to go with the exception of STS. Let me know about the status of that and we can proceed from there.

Thanks!
Matt
One final thing - I'm not sure if this is expected behavior, but I was able to edit any event, even ones I didn't create.
(In reply to Matt Fuller :mfuller from comment #16)
> One final thing - I'm not sure if this is expected behavior, but I was able
> to edit any event, even ones I didn't create.

Yes Reps (i.e. logged in users in the 'Rep' group) can edit all events.
Hi Giorgos, thanks - everything is good to go once the STS issue is addressed.
Assignee: yboily → mfuller
Resolving, STS will track in next release.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.