Last modified: 2014-08-14 18:59:10 UTC
Security review request for FundraisingChart extension. Depends on bug #65834
Any update for this? Fundraising is hoping to use this extension for our fiscal year-end report, which will be published at the end of August. Thanks!
For some reason this was assigned to fr-tech and not Chris. That's probably a cause for delay :)
What domain is this going to be deployed on?
(In reply to Chris Steipp from comment #3) > What domain is this going to be deployed on? collab and meta
Hi Sherah, It's a little difficult to do a thorough review because I keep hitting bugs with the version in gerrit, but there are a couple of design issues and some of the ways you're doing things I think are likely going to lead to security issues. Let me know if you want to schedule time to talk through these. FundraisingChart.body.php * Urls should pull from https, or protocol relative links so https views aren't compromised * Not security but.. ** Adding bootstrap.css is messing with the page styles ** Map charts mess up the page formatting because of stray <tr> tags resources/js/fundraising_charts.js * There's a bad privacy leak in that you're not validating the urls you pull in from user-controlled tags on the page. A user can add wikitext to the page like "<div class="pieArea" id="Something" data-chartdata="http://www.google.com/">12345</div>" and your code will call out to google. ** Because of that, there a few places that you're writing out modules/ext.fundraisingChart.datamaps/datamaps.world.js * addLegend() - concatting html is usually bad. Either need to demonstrate sanitization or use dom objects. * updatePopup() - Setting .html() is dangerous. Sanitize before passing to popupTemplate, or in the template function. You should document where the sanitization is happening either way. I'm still working on d3. I think it should be fine, but I noticed the included version is very old. Is someone going to keep it updated in case there are security issues in the library?
(In reply to Chris Steipp from comment #5) > I'm still working on d3. I think it should be fine, but I noticed the > included version is very old. Is someone going to keep it updated in case > there are security issues in the library? D3 looks like it's pretty sane about how it does what it does. Should be fine to use.
