GSA / datagov-ckan-multi

Other
10 stars 6 forks source link

Resolve SQLAlchemy dependency vulnerabilities #553

Closed adborden closed 3 years ago

adborden commented 3 years ago

_Please keep any sensitive details in Google Drive._

Date of report: 9/10/2020 Severity: high Due date: 10/10/2020

Due date is based on severity and described in RA-5. 15-days for Critical, 30-days for High, and 90-days for Moderate and lower.

* When a finding is identified, we create two issues. One to address the specific instance identified in the report. The other is to identify and address all other occurrences of this vulnerability within the application.

Brief description

https://snyk.io/vuln/SNYK-PYTHON-SQLALCHEMY-590109 https://github.com/GSA/catalog.data.gov/issues/112

https://snyk.io/vuln/SNYK-PYTHON-SQLALCHEMY-173678 https://github.com/GSA/catalog.data.gov/issues/119

adborden commented 3 years ago

If upgrading SQLAlchemy is not an option, we'll have to identify an alternative https://github.com/GSA/datagov-deploy/wiki/Dependency-scanning#triage-walkthrough

adborden commented 3 years ago

I've lumped two sqlalchemy issues in here together, but we should split them out if the work is different.

thejuliekramer commented 3 years ago

@FuhuXia @jbrown-xentity did you have to make change for this vulnerability in catalog classic?

thejuliekramer commented 3 years ago

@avdata99 is going to reach out to the CKAN community to see if anyone has taken steps to tackle this in older versions of CKAN

avdata99 commented 3 years ago

Following here and internally with my team

adborden commented 3 years ago

FYI, any exceptions/resolutions we make are documented in the .snyk file. For SNYK-PYTHON-SQLALCHEMY-173678, we determined that CKAN 2.3 did not use the group_by or order_by parameters and therefore no fix is needed.

Oddly, SNYK-PYTHON-SQLALCHEMY-590109 lists a similar exception, but that doesn't make sense regarding the vulnerability description. @pjsharpe07 are you sure that one is correct?

adborden commented 3 years ago

One option we discussed was to fork SQLAlchemy and apply this patch https://github.com/sqlalchemy/sqlalchemy/commit/d3ba45c00f83f7d57be5fb5caa7fb27f61624e28 for https://snyk.io/vuln/SNYK-PYTHON-SQLALCHEMY-590109

avdata99 commented 3 years ago

Moving to TODO since we postposed this