fix: long version of go dependency unable to inserted into software table (CVE-2020-36569) by VSrinivasaRaghavan · Pull Request #5221 · dependency-check/DependencyCheck

VSrinivasaRaghavan · 2023-01-06T07:31:05Z

Fixes Issue

Fix: long version of go dependency unable to inserted into software table (CVE-2020-36569)

Description of Change

Alter's column size to varchar 75

Please add a description of the proposed change
Alters to a minimum possible value of version columns to 75 characters. Could be more or less as well.

Have test cases been added to cover the new functionality?

no

gregoranders · 2023-01-06T07:50:36Z

commit message should start with a lowercase fix otherwise the pipeline breaks @srini00088

vignesh-gupta-xom

Hey there, I believe this would be really help for one of the current issues i.e. issue with 'CVE-2020-36569' without harming any other.

…able (CVE-2020-36569)

VSrinivasaRaghavan · 2023-01-06T08:06:18Z

@gregoranders thanks, have amended the commit message

vignesh-gupta-xom · 2023-01-06T08:11:02Z

@srini00088 please add space and commit is with small letter.

vignesh-gupta

@srini00088 kudos to you great job. Proactively solves the issue. I believe this will solve the issue

gpx-jonathan-amos · 2023-01-06T08:41:24Z

Any tentative timelines on when this fix can get released? It's critical.

marcelstoer · 2023-01-06T09:42:16Z

AFAIU this ain't complete yet. You need to modify dependencycheck.properties as per https://github.com/jeremylong/DependencyCheck/blob/main/core/src/main/resources/dependencycheck.properties#L21-L24.

wilkovanderveen

.

Nacadamia · 2023-01-06T10:16:14Z

How about a longer field than 75. Lets crank it up to at least 100 to buy some time.

…ze scripts and properties

Nacadamia · 2023-01-06T10:50:05Z

@@ -0,0 +1,6 @@
+ALTER TABLE software ALTER COLUMN versionEndExcluding SET DATA TYPE VARCHAR(75);


I'd suggest a higher number than 75, because it is the exact the size for this specific entry.

How about 255 like other COLUMNs or even more. Is their a concret reason for 75 except that the current CVE needs 75?

I am all for 255, at least I don't see a reason to not make it reasonably bigger, otherwise 75 is just us waiting for the next one to happen.

marcelstoer · 2023-01-06T10:53:58Z

+ALTER TABLE software ALTER COLUMN versionEndExcluding SET DATA TYPE VARCHAR(75);
+ALTER TABLE software ALTER COLUMN versionEndIncluding SET DATA TYPE VARCHAR(75);
+ALTER TABLE software ALTER COLUMN versionStartExcluding SET DATA TYPE VARCHAR(75);
+ALTER TABLE software ALTER COLUMN versionStartIncluding SET DATA TYPE VARCHAR(75);


Suggested change

ALTER TABLE software ALTER COLUMN versionEndExcluding SET DATA TYPE VARCHAR(75);

ALTER TABLE software ALTER COLUMN versionEndIncluding SET DATA TYPE VARCHAR(75);

ALTER TABLE software ALTER COLUMN versionStartExcluding SET DATA TYPE VARCHAR(75);

ALTER TABLE software ALTER COLUMN versionStartIncluding SET DATA TYPE VARCHAR(75);

ALTER TABLE software ALTER COLUMN versionEndExcluding SET DATA TYPE VARCHAR(100);

ALTER TABLE software ALTER COLUMN versionEndIncluding SET DATA TYPE VARCHAR(100);

ALTER TABLE software ALTER COLUMN versionStartExcluding SET DATA TYPE VARCHAR(100);

ALTER TABLE software ALTER COLUMN versionStartIncluding SET DATA TYPE VARCHAR(100);

I'd say 255 is even more on the safe side, as @nusmanov suggested:

ottlinger · 2023-01-06T11:14:25Z

Wouldn't it be safer to trim any data read from CVE to not risk database length violations? Otherwise any faulty data in CVE entries will make the plugin fail again.

nusmanov · 2023-01-06T11:48:10Z

@@ -0,0 +1,6 @@
+ALTER TABLE software ALTER COLUMN versionEndExcluding SET DATA TYPE VARCHAR(75);


as @Keymaster65 already suggested, i would also prefer to take the column length of 255 chars:

Suggested change

ALTER TABLE software ALTER COLUMN versionEndExcluding SET DATA TYPE VARCHAR(75);

ALTER TABLE software ALTER COLUMN versionEndExcluding SET DATA TYPE VARCHAR(255);

ALTER TABLE software ALTER COLUMN versionEndIncluding SET DATA TYPE VARCHAR(255);

ALTER TABLE software ALTER COLUMN versionStartExcluding SET DATA TYPE VARCHAR(255);

ALTER TABLE software ALTER COLUMN versionStartIncluding SET DATA TYPE VARCHAR(255);

lc-nyovchev · 2023-01-06T11:57:43Z

@@ -0,0 +1,6 @@
+ALTER TABLE software ALTER COLUMN versionEndExcluding SET DATA TYPE VARCHAR(75);


I am all for 255, at least I don't see a reason to not make it reasonably bigger, otherwise 75 is just us waiting for the next one to happen.

lc-nyovchev · 2023-01-06T11:58:39Z

+ALTER TABLE software ALTER COLUMN versionEndExcluding SET DATA TYPE VARCHAR(75);
+ALTER TABLE software ALTER COLUMN versionEndIncluding SET DATA TYPE VARCHAR(75);
+ALTER TABLE software ALTER COLUMN versionStartExcluding SET DATA TYPE VARCHAR(75);
+ALTER TABLE software ALTER COLUMN versionStartIncluding SET DATA TYPE VARCHAR(75);


I'd say 255 is even more on the safe side, as @nusmanov suggested:

jjoshm · 2023-01-06T13:11:23Z

suggest that this be merged as a hotfix asap so that it works for now and be optimized and discussed afterwards.
The complexity seems manageable.

jeremylong · 2023-01-06T13:22:44Z

I will update this PR and release an update today.

vincent-petit

I wonder that you should also update the type in the create table software in the initialize[_xxx].sql
https://github.com/jeremylong/DependencyCheck/blob/afc69a0de9e85aca3c9d5ac3f0c19057cecc6336/core/src/main/resources/data/initialize.sql#L31

I don't know the project, but when these columns have been added to the table with the script upgrade_5.1.sql https://github.com/jeremylong/DependencyCheck/blob/21b08b3b8368e224beeca346cfaa246ea8bb3ac9/core/src/main/resources/data/upgrade_5.1.sql#L1, they have been added in the create table too

pijean-rca · 2023-01-06T14:39:27Z

@@ -0,0 +1,6 @@
+ALTER TABLE software ALTER COLUMN versionEndExcluding SET DATA TYPE VARCHAR(75);


Should not this file be named upgrade_5.2.1.sql ?

pijean-rca · 2023-01-06T14:50:03Z

 ### if you increment the DB version then you must increment the database file path
 ### in the mojo.properties, task.properties (maven and ant respectively), and
 ### the gradle PurgeDataExtension.
-data.version=5.2.1


This property is used to choose the update file (org.owasp.dependencycheck.data.nvdcve.DatabaseManager#DB_STRUCTURE_UPDATE_RESOURCE) when h2 is used (org.owasp.dependencycheck.data.nvdcve.DatabaseManager#updateSchema).

Analysis crashes when the upgrade file does not exist. Along with my previous comment I would say that this property should be equal to 5.2.1

It's not this property, but the database version property in your H2 database that governs which upgrade SQL is searched for. This property governs the expected database version (and if actual is lower than that will search for upgrade scripts to update the H2 database

95Kevid

Looks good to me

File must be named according to the database-version it upgrades.

aikebah · 2023-01-06T16:41:34Z

I will update this PR and release an update today.

@jeremylong did a fix-up of the update-script name, before spotting your response. Will leave the further polishing to you.

A-Fitz-Nelnet · 2023-01-06T16:58:11Z

Does utils\src\test\resources\dependencycheck.properties also need to be updated?

jeremylong · 2023-01-06T17:00:01Z

see jeremylong/DependencyCheck#5229

ltaboada1 · 2023-01-06T18:36:40Z

Could you please upgrade gradle plugin to 7.4.4 as well? Latest is still 7.4.3 https://plugins.gradle.org/plugin/org.owasp.dependencycheck

marcthayer · 2023-01-06T22:12:34Z

Could you please upgrade gradle plugin to 7.4.4 as well? Latest is still 7.4.3 https://plugins.gradle.org/plugin/org.owasp.dependencycheck

Looks like it's showing 7.4.4 now.

DominicTulley · 2023-01-09T08:04:44Z

@jeremylong could you update the jenkins plugin too please?

sreekanthops · 2023-01-09T08:18:39Z

@jeremylong same here, please update this to jenkins please

sreekanthops · 2023-01-09T09:00:30Z

Thanks, I could see latest 7.4.4 in dependency check plugin

DominicTulley · 2023-01-09T09:37:28Z

Ah, I missed that. Maybe the plugin didn't need updating. Excellent.

boring-cyborg Bot added the core changes to core label Jan 6, 2023

VSrinivasaRaghavan changed the title ~~Fix: long version of go dependency unable to inserted into software table (CVE-2020-36569)~~ fix: long version of go dependency unable to inserted into software table (CVE-2020-36569) Jan 6, 2023

VSrinivasaRaghavan mentioned this pull request Jan 6, 2023

DatabaseException: Error updating 'CVE-2020-36569' #5220

Closed

Fritzybois approved these changes Jan 6, 2023

View reviewed changes

vignesh-gupta-xom approved these changes Jan 6, 2023

View reviewed changes

fix: long version of go dependency unable to inserted into software t…

e5a2f29

…able (CVE-2020-36569)

RajNirmal force-pushed the fix/CVE-2020-36569 branch from a875ecd to e5a2f29 Compare January 6, 2023 08:04

vignesh-gupta-xom approved these changes Jan 6, 2023

View reviewed changes

vignesh-gupta approved these changes Jan 6, 2023

View reviewed changes

jgavignet approved these changes Jan 6, 2023

View reviewed changes

pvroegh approved these changes Jan 6, 2023

View reviewed changes

wilkovanderveen approved these changes Jan 6, 2023

View reviewed changes

fix: update data version to 5.2.2 and update it in all other initiali…

c4442e9

…ze scripts and properties

boring-cyborg Bot added tests test cases utils changes to utils labels Jan 6, 2023

Nacadamia reviewed Jan 6, 2023

View reviewed changes

marcelstoer suggested changes Jan 6, 2023

View reviewed changes

divertitoS approved these changes Jan 6, 2023

View reviewed changes

kaby2201 approved these changes Jan 6, 2023

View reviewed changes

jgavignet approved these changes Jan 6, 2023

View reviewed changes

nusmanov reviewed Jan 6, 2023

View reviewed changes

nusmanov approved these changes Jan 6, 2023

View reviewed changes

lc-nyovchev approved these changes Jan 6, 2023

View reviewed changes

nusmanov approved these changes Jan 6, 2023

View reviewed changes

catap suggested changes Jan 6, 2023

View reviewed changes

Comment thread core/src/main/resources/data/upgrade_5.3.sql

ThomasVerhoeven1998 approved these changes Jan 6, 2023

View reviewed changes

jeremylong added this to the 7.4.4 milestone Jan 6, 2023

vincent-petit reviewed Jan 6, 2023

View reviewed changes

fransah approved these changes Jan 6, 2023

View reviewed changes

pijean-rca reviewed Jan 6, 2023

View reviewed changes

meridsa approved these changes Jan 6, 2023

View reviewed changes

95Kevid approved these changes Jan 6, 2023

View reviewed changes

Rename upgrade_5.3.sql to upgrade_5.2.1.sql

315c0ba

File must be named according to the database-version it upgrades.

srcimon approved these changes Jan 6, 2023

View reviewed changes

jeremylong mentioned this pull request Jan 6, 2023

fix: long version of go dependency unable to inserted into software table (CVE-2020-36569) #5229

Merged

jeremylong removed this from the 7.4.4 milestone Jan 6, 2023

jeremylong closed this Jan 6, 2023

github-actions Bot locked as resolved and limited conversation to collaborators Dec 17, 2024

		@@ -0,0 +1,6 @@
		ALTER TABLE software ALTER COLUMN versionEndExcluding SET DATA TYPE VARCHAR(75);

-ALTER TABLE software ALTER COLUMN versionEndExcluding SET DATA TYPE VARCHAR(75);
+ALTER TABLE software ALTER COLUMN versionEndExcluding SET DATA TYPE VARCHAR(255);
+ALTER TABLE software ALTER COLUMN versionEndIncluding SET DATA TYPE VARCHAR(255);
+ALTER TABLE software ALTER COLUMN versionStartExcluding SET DATA TYPE VARCHAR(255);
+ALTER TABLE software ALTER COLUMN versionStartIncluding SET DATA TYPE VARCHAR(255);

Conversation

VSrinivasaRaghavan commented Jan 6, 2023

Fixes Issue

Description of Change

Have test cases been added to cover the new functionality?

Uh oh!

gregoranders commented Jan 6, 2023

Uh oh!

vignesh-gupta-xom left a comment

Choose a reason for hiding this comment

Uh oh!

VSrinivasaRaghavan commented Jan 6, 2023

Uh oh!

vignesh-gupta-xom commented Jan 6, 2023

Uh oh!

vignesh-gupta left a comment

Choose a reason for hiding this comment

Uh oh!

gpx-jonathan-amos commented Jan 6, 2023

Uh oh!

marcelstoer commented Jan 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wilkovanderveen left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nacadamia commented Jan 6, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ottlinger commented Jan 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jjoshm commented Jan 6, 2023

Uh oh!

jeremylong commented Jan 6, 2023

Uh oh!

vincent-petit left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pijean-rca Jan 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pijean-rca Jan 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

95Kevid left a comment

Choose a reason for hiding this comment

Uh oh!

aikebah commented Jan 6, 2023

Uh oh!

A-Fitz-Nelnet commented Jan 6, 2023

Uh oh!

jeremylong commented Jan 6, 2023

Uh oh!

ltaboada1 commented Jan 6, 2023

Uh oh!

marcthayer commented Jan 6, 2023

Uh oh!

marcelstoer commented Jan 6, 2023 •

edited

Loading

wilkovanderveen left a comment •

edited

Loading

ottlinger commented Jan 6, 2023 •

edited

Loading

vincent-petit left a comment •

edited

Loading

pijean-rca Jan 6, 2023 •

edited

Loading

pijean-rca Jan 6, 2023 •

edited

Loading