Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete unused ProteinArraySignificanceTest endpoint with security issue #8751

Merged
merged 1 commit into from Jul 20, 2021

Conversation

adamabeshouse
Copy link
Contributor

Fixes #8680

@jjgao jjgao temporarily deployed to cbioportal-8680-securit-r8oubm July 15, 2021 16:16 Inactive
Copy link
Contributor

@sheridancbio sheridancbio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this servlet is being removed, I think we also need to drop the servlet beans from portal/src/main/webapp/WEB-INF/web.xml (lines 341 - 348). Otherwise I expect we would see a BeanNotFound Exception during startup ... or maybe when the servlet is first accessed.

From the issue comments it seems like there is already approval to drop this service from the core. If that is so, this is a first step towards our larger cleanup (jettisoning the core).

@ghost
Copy link

ghost commented Jul 15, 2021

If I may suggest. Keep the class. Make processRequest return empty result early if it is really not used. That would be the quickest less intrusive security fix without making too much refactoring.

@n1zea144
Copy link
Contributor

@adamabeshouse Why not just include the update to web.xml as Rob suggests? Seems like an easy update to the PR.

Signed-off-by: Adam Abeshouse <abeshoua@mskcc.org>
@jjgao jjgao temporarily deployed to cbioportal-8680-securit-r8oubm July 16, 2021 15:43 Inactive
@adamabeshouse
Copy link
Contributor Author

@n1zea144 @sheridancbio done! thanks

@sonarcloud
Copy link

sonarcloud bot commented Jul 16, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@n1zea144 n1zea144 self-requested a review July 20, 2021 00:50
@adamabeshouse adamabeshouse merged commit a4f94c0 into master Jul 20, 2021
10 of 11 checks passed
@adamabeshouse adamabeshouse deleted the 8680-security-fix branch July 20, 2021 15:06
@ghost ghost mentioned this pull request Aug 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SECURITY] Denial of service because of unsafe regex processing
5 participants