NCSC-NL / taranis3

Taranis
Other
59 stars 17 forks source link

Incorrect SQL query when version equal to "-" #55

Open MariusUrkis opened 2 years ago

MariusUrkis commented 2 years ago

https://github.com/NCSC-NL/taranis3/blob/25672371304ccf35a6f7b6246f70ea3b11273bf5/pm/Taranis/Publish.pm#L244

If Software version is omitted by setting version value to '-', the next query L247 returns nothing. That results in incorrect query at the line 260 (with empty set "IN ( )"). 2022-06-16 13:27:25.204 UTC [2451141] taranis@taranis ERROR: syntax error at or near ")" at character 163 2022-06-16 13:27:25.204 UTC [2451141] taranis@taranis STATEMENT: SELECT distinct cg.id FROM constituent_group AS cg INNER JOIN soft_hard_usage AS shu ON ( cg.id = shu.group_id ) WHERE ( cg.status = $1 AND shu.soft_hard_id IN ( ) )

Possible fix: $where->{version} = [undef, '-', '' ] if $ware->{version};

markov2 commented 2 years ago

If I am not mistaken, the error is produced in _getConstituentGroupsForWareList() when there are no matching $soft_hard_ids. So it seems logical for me to add this:

sub _getConstituentGroupsForWareList {
    my ($soft_hard_ids) = shift;
   @$soft_hard_ids or return [];    # <---

    return [ Database->simple->select(...) ];

Why do you think - should be interpreted as a valid alternative for unspecified? Isn't that a new feature?

MariusUrkis commented 2 years ago

What I see in db after cpe dictionary update is: taranis=# select id, name, version from software_hardware where (deleted = 'f' and name ilike 'Windows%' and producer = 'microsoft' ); id | name | version -----------+------------------------------------------------------------------+--------- 360056132 | windows 2000_beta3 | - 360056139 | Windows 8.1 x86 (32-bit) | -

Didn't try to trace deeper actually

markov2 commented 2 years ago

It is quite stupid: those all look like separate versions from Windows but are not registered that way.

We are not using the CPE definitions anymore, because there are simply too many: we only add products by hand. Therefore, this was not discovered before. I'll fix this in the upcoming release. Your fix sounds sufficient.