gigascience / gigadb-website

Source code for running GigaDB
http://gigadb.org
GNU General Public License v3.0
9 stars 14 forks source link

Cross-Site Scripting XSS attacks #343

Open jessesiu opened 5 years ago

jessesiu commented 5 years ago

The current yii framework has XSS attacks problem. On GigaDB website, when embedded the script on URL e.g.

屏幕快照 2019-09-02 下午1 25 19

I temporarily fixed it using the CHtmlPurifier https://www.yiiframework.com/doc/api/1.1/CHtmlPurifier for those two controllers. But we need to think about how to protect XSS attacks for all controllers URLs in GigaDB. Maybe add the filter for URL management to filter all scripts in the URL? or apply CHtmlPurifer to all exchange content.

pli888 commented 5 years ago

@rija BGI IT department has been checking the security of GigaDB and found that it is vulnerable to cross-site scripting attacks. @jessesiu has provided a temporary fix but can you suggest how we can implement a sitewide solution?

rija commented 5 years ago

Hi @pli888, @jessesiu,

The most effective security strategy is "defense in depth", wich means for a given risk, that at each layer of the application stack, the necessary best practices are applied against the risk.

@jessesiu's change should not be seen as just temporary, I reckon. As he last suggested, It should be done for all controllers as it's the best practice for the layer where dynamic data need to be displayed.

That said, the first layer is the web browser, we need to tell it what is legit and what is not regarding content that will be executed on the client. This is where Content Security Policy (CSP) is useful as the first line of defense ; Its application is global and filter based. It was one of my recommendations out of the code audit.

I realised later that the problem of implementing CSP effectively with respect to GigaDB is that the combination of: not using HTTPS, diverse ways of integrating third party services and the legacy frontend code/practice, make it hard to enact a working CSP policy. An effective CSP policy turned on in nginx would break many functionalities on GigaDB as it is ; prior, potentially significant work would need to be done.

What I recommend is taking a gradual approach to implementation: First enable HTTPS and a strict CSP policy in a report-only mode (so the violations will be found but won't block functionalities), then release it to live environment, monitor for awhile, and for each violation reported, create a github issue.

Next allocate a small percentage of every work period to tackle technical debt (so not to take all the time from delivering new features), and then periods after periods, work on those issues like a pipeline of work, aiming at not having too much Work In Process at any given time. Over time, this will harden the web site and reduce the amount of security-related unplanned work.

Note 1:

The SAST and DAST steps in the CI/CD pipeline can find automatically many potential security issues, including XSS. Actually the last SAST report shows other potential risks with dynamic data display. (https://gitlab.com/gigascience/upstream/gigadb-website/-/jobs/220346390) It's a good idea to have them presented in a way so to incite us at keeping looking at them. We cannot really automate decisions based on them because risk profile is site specific and false positives exist but at least we can make daily practice keeping an eyeballs on them to make appropriate decisions.

Note 2:

By itself, the issue reported here on the search field is fairly innocuous to GigaDB (though It can be combined with other vulnerabilies). It would be more problematic if it happened on a form field that save data to the database. That's the data layer where form field validation is the third line of defense. Frameworks (including Yii, even the old Yii 1.1) have good tools to help securing that layer (e.g: custom validators). The use of PHP7+'s strong typing pertain to that line too, especially for the DAO classes that use less framework facilities.