Closed Bug 797857 Opened 13 years ago Closed 12 years ago

SecReview: django-badger and badg.us

Categories

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

Product:
mozilla.org
Component:
Security Assurance: Review Request
Type:
task

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: lorchard, Assigned: st3fan)

References

Details

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

> Who is/are the point of contact(s) for this review? Les Orchard (lorchard@mozilla.com) > Please provide a short description of the feature / application > (e.g. problem solved, use cases, etc.): * django-badger is a Django app that supports managing and awarding badges, it also aims to support the Mozilla Foundation's Open Badge Infrastructure (OBI) * badg.us is a Playdoh-based site that uses django-badger to provide a general badge service, allowing users to manage and award their own set of badges The first goal, here, is to get badg.us hosted on Mozilla infrastructure. The second goal is to incorporate django-badger into further Mozilla sites (like MDN or SUMO), in order to create and offer site-specific badges from each of those projects. > Please provide links to additional information (e.g. feature page, wiki) > if available and not yet included in feature description: Both django-badger and badg.us can be found on github: * https://github.com/lmorchard/django-badger * https://github.com/lmorchard/badg.us Running instances of badg.us can be found here: * http://badg.us/ (running on a non-mozilla server) * http://badgus-dev.allizom.org/ (mozilla dev server) * http://badgus.allizom.org/ (mozilla staging server) For more information on badges in general: * http://decafbad.com/2010/07/badger-article/ * http://openbadges.org/en-US/ > Does this request block another bug? If so, please > indicate the bug number Nothing blocked by this, so far. > This review will be scheduled amongst other requested > reviews. What is the urgency or needed completion date > of this review? I don't think the urgency is very high. > To help prioritize this work request, does this project > support a goal specifically listed on this quarter's > goal list? If so, which goal? mrz has written in email, "This is now a Q4 goal for my team." But, I don't have a link to this in any goal lists. > Does this feature or code change affect Firefox, > Thunderbird or any product or service the Mozilla ships > to end users? Not as far as I know. > Are there any portions of the project that interact > with 3rd party services? Yes. Badge awards on badg.us can be shared on Twitter/Facebook/Google+. Also, the MoFo OBI allows awarded badges to be posted to a personal backpack. So far, the backpack is mozilla-hosted, but the infrastructure will someday allow for distributed backpacks. Also, badg.us uses Persona / BrowserID logins, which could potentially authenticate through 3rd party providers. > Will your application/service collect user data? > If so, please describe Yes. General profile information (eg. name, email, bio, homepage) and it will track badges created, awarded, and received. > 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. No date in mind, just want to get the process started
Assignee: nobody → amuntner
Whiteboard: [pending secreview] → [pending secreview][start yyyy-mm-dd][target yyyy-mm-dd]
I know this is super low-priority, but is there any sense at all of timing for this? This is currently running on my servers, and I'm hoping to get it onto Mozilla infrastructure rather than shut it down
Adam, what is your schedule for this looking like?
Flags: needinfo?(amuntner)
I would love to work on moving this to the new Mozilla theme if nothing else.
In terms of priority, having a badges infrastructure is a priority for the community building work we're doing with the Stewards program so it would be great to see this move forward. I've been talking with the Webdev Stewards lately about how we can use badges (and other forms of recognition) to grow the number of volunteers contributing to webdev projects. Luke has already identified specific contribution activities and the associated badges at: https://wiki.mozilla.org/Contribute/Conversion_points#Webdev I talked with Emily from the Open Badges project at the Foundation about this recently too and she's interested in the idea of doing a badges pilot with webdev that could serve as a template for a general badges plan for contributors across Mozilla.
Can we get an estimate on when we'll get a time estimate for this bug? :)
I'm helping out with this one. Starting today with the badg.us site. I'll get it running on a VM to poke at it.
There is an injection issue with the Bio field of the user profile. I was able to set it to: I like <b>Cheese</b> yo <a href="javascript:alert(1)">click me</a> Which was included in the profile page without being escaped. I was also able to set the my name to <h1>Stefan</h1>. Although that is properly escaped when displayed I think it is safer to simply not allow characters like <> in a field like that.
Created a new badge with html tags in the name, tags and description. I think something went wrong because the following page just shows up blank: https://badgus.allizom.org/en-US/badges/badge/h1cheeseh1 Could be an issue of input validation. I think it needs to be more strict.
The site references https://browserid.org/include.js which has been deprecated. The correct place to be is https://login.persona.org/include.js This is likely a library issue. And not a high security risk but should probably be fixed anyway.
The production server runs something on port 9999 that seems to be powered by http://expressjs.com/ ... is this documented anywhere? Is this something that needs a review?
(In reply to Stefan Arentz [:st3fan] from comment #10) > The production server runs something on port 9999 that seems to be powered > by http://expressjs.com/ ... is this documented anywhere? Is this something > that needs a review? Er... what production server? There is none on Mozilla hardware, yet. If you mean badg.us, that's my own server where I run hobby projects. Don't test there.
(In reply to Stefan Arentz [:st3fan] from comment #9) > The site references https://browserid.org/include.js which has been > deprecated. The correct place to be is https://login.persona.org/include.js > > This is likely a library issue. And not a high security risk but should > probably be fixed anyway. This project has been pretty much frozen since before that switch over. Mostly waiting to make any changes until after a sec-review is done, and I have more time to work on it :)
(In reply to Les Orchard [:lorchard] from comment #11) > (In reply to Stefan Arentz [:st3fan] from comment #10) > > The production server runs something on port 9999 that seems to be powered > > by http://expressjs.com/ ... is this documented anywhere? Is this something > > that needs a review? > > Er... what production server? There is none on Mozilla hardware, yet. If you > mean badg.us, that's my own server where I run hobby projects. Don't test > there. I'm confused now because badg.us sure looks like an official deployment.
(In reply to Stefan Arentz [:st3fan] from comment #7) > There is an injection issue with the Bio field of the user profile. I was > able to set it to: > > I like <b>Cheese</b> yo <a href="javascript:alert(1)">click me</a> > > Which was included in the profile page without being escaped. This should be fixed now - HTML is still allowed, but it's filtered through the Bleach module: https://github.com/lmorchard/badg.us/commit/299420b73a4ead6244a4d2e643359f2cdd86ec3f I haven't got the keys to update badgus.allizom.org, but it should appear on badgus-dev in a little while.
(In reply to Stefan Arentz [:st3fan] from comment #13) > I'm confused now because badg.us sure looks like an official deployment. See above, in comment #0 - "* http://badg.us/ (running on a non-mozilla server)" What looks official about it? There's no mozilla branding, etc. Getting through a sec-review is on the road to getting it official, but I'm not there yet. badg.us is my own server, started before any deployments on Mozilla hardware to get the ball rolling on this project
(In reply to Les Orchard [:lorchard] from comment #15) > badg.us is my own server, started before any deployments on Mozilla hardware > to get the ball rolling on this project (In other words, this has mostly been my own personal project up to now, and we're trying to get it adopted by mozilla)
Thanks for the clarification Les. Makes sense :-)
(In reply to Stefan Arentz [:st3fan] from comment #17) > Thanks for the clarification Les. Makes sense :-) Also, thank you very much for looking at this stuff!
(In reply to Stefan Arentz [:st3fan] from comment #8) > Created a new badge with html tags in the name, tags and description. I > think something went wrong because the following page just shows up blank: > > https://badgus.allizom.org/en-US/badges/badge/h1cheeseh1 > > Could be an issue of input validation. I think it needs to be more strict. Yeah, turns out there was an error when tags containing slashes are used. Quick bugfix here: https://github.com/lmorchard/django-badger/commit/12e106c60dad84b141db3b198b0e9ddbffb8c186 Also filed a github issue to further consider filtering out tags that look like HTML: https://github.com/lmorchard/django-badger/issues/129
(In reply to Stefan Arentz [:st3fan] from comment #9) > The site references https://browserid.org/include.js which has been > deprecated. The correct place to be is https://login.persona.org/include.js > > This is likely a library issue. And not a high security risk but should > probably be fixed anyway. Oh, also, I didn't mean to just leave a snarky comment. :( To follow up, I filed a new github issue: https://github.com/lmorchard/django-badger/issues/130
(In reply to Les Orchard [:lorchard] from comment #20) > (In reply to Stefan Arentz [:st3fan] from comment #9) > > The site references https://browserid.org/include.js which has been > > deprecated. The correct place to be is https://login.persona.org/include.js > > > > This is likely a library issue. And not a high security risk but should > > probably be fixed anyway. > > Oh, also, I didn't mean to just leave a snarky comment. :( To follow up, I > filed a new github issue: > > https://github.com/lmorchard/django-badger/issues/130 No worries. I'm also going to file bugs against BrowserID integration libraries this week to get them updated.
Risk/Priority Ranking Exercise https://wiki.mozilla.org/Security/RiskRatings Priority: 2 (P4) - Team Quarterly Goal Operational: 5 - Blocker User: 3 - Major Privacy: 3 - Major Engineering: 3 - Major Reputational: 3 - Major Priority Score: 27
Flags: needinfo?(amuntner)
Whiteboard: [pending secreview][start yyyy-mm-dd][target yyyy-mm-dd] → [pending secreview][start yyyy-mm-dd][target yyyy-mm-dd][Score:27:Medium]
Priority: -- → P4
> (In reply to Stefan Arentz [:st3fan] from comment #9) > > The site references https://browserid.org/include.js which has been > > deprecated. The correct place to be is https://login.persona.org/include.js > > > > This is likely a library issue. And not a high security risk but should > > probably be fixed anyway. This should be fixed now: https://github.com/lmorchard/django-badger/issues/130#issuecomment-11589035 It should appear on my badg.us server and http://badgus-dev.allizom.org, but I can't update badgus.allizom.org because I don't have access to do so (pending bug 797468) So, I think I've addressed comment 7, comment 8, and comment 9. Were there any other issues?
Depends on: 797468
No I don't think there were any other issues. Did you check the input validation for all fields in all forms?
(In reply to Stefan Arentz [:st3fan] from comment #24) > No I don't think there were any other issues. Did you check the input > validation for all fields in all forms? Hmm, I don't think there were any input validation problems, though I did file this issue to consider restricting the characters allowed in tags on badges: https://github.com/lmorchard/django-badger/issues/129 But, that shouldn't be an XSS issue since all the places where tags are displayed should have the output escaped.
Ping - what's left to close out this review?
(In reply to Les Orchard [:lorchard] from comment #26) > Ping - what's left to close out this review? Anyone? Is there anything I can to do wrap this up?
Les, I will sign off on this later today.
Assignee: amuntner → sarentz
Les, I think we are good. Unless you introduced substantial new functionality or big code changes since december?
(In reply to Stefan Arentz [:st3fan] from comment #29) > Les, I think we are good. Unless you introduced substantial new > functionality or big code changes since december? Nope, other than some cleanup and tests, I've been holding off on anything interesting. I'll just close this & let someone reopen it if necessary :)
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Blocks: 835934
You need to log in before you can comment on or make changes to this bug.