Closed exploide closed 2 years ago
Hello @exploide . Thanks again for your contributions.
I added some comments because there are some lines that I am not sure how they will work. There are mostly 2 things:
I recall RedHat using /etc/redhat-release
instead of os-release
in the past so using blindly os-release
might break things in older RedHat versions (not sure how old though). Did you test this in any way? Are you aware of the use of redhat-release
? Don't get me wrong, I think having your changes is better than not having them but I am wondering if we could avoid some false positives or just improve them.
The function lse_get_package_version
might not get the version. For example the package name might change among distros or the way of retrieving it in some distro does not work properly. For this reason I think it is better to not blindly use $package_version
and either check that it is not empty or use the version from a different source (like commant --version
)
Thanks again
I recall RedHat using /etc/redhat-release instead of os-release in the past so using blindly os-release might break things in older RedHat versions (not sure how old though). Did you test this in any way? Are you aware of the use of redhat-release? Don't get me wrong, I think having your changes is better than not having them but I am wondering if we could avoid some false positives or just improve them.
As a long time Fedora user I know that /etc/redhat-release
exists but I have no clue since when /etc/os-release
has been added to RHEL. I assumed it is there long enough for our purposes. Unfortunately I cannot test the changes because I don't have access to a RHEL system. Do you think the current implementation makes anything worse compared with not having the changes? Not sure how to improve this. I mean it's already checking the existence of this file before reading it. And the tests for rocky and centos which are already present should suffer from the same problem (ok, maybe not rocky because it doesn't exist for so long).
The function lse_get_package_version might not get the version. For example the package name might change among distros or the way of retrieving it in some distro does not work properly. For this reason I think it is better to not blindly use $package_version and either check that it is not empty or use the version from a different source (like commant --version)
Are you referring to the "Vulnerable! ... version" output? The reason I changed this is because it makes it easier to double check afterwards if this is a false positive. Especially when you cannot run further commands (I only got the output of lse for review...). But you are right. Maybe I should check whether the variable is empty and use the command version then?
As a long time Fedora user I know that
/etc/redhat-release
exists but I have no clue since when/etc/os-release
has been added to RHEL. I assumed it is there long enough for our purposes. Unfortunately I cannot test the changes because I don't have access to a RHEL system. Do you think the current implementation makes anything worse compared with not having the changes? Not sure how to improve this. I mean it's already checking the existence of this file before reading it. And the tests for rocky and centos which are already present should suffer from the same problem (ok, maybe not rocky because it doesn't exist for so long).
I guess we can leave it like this for now. It is certainly better that not have anything :)
Are you referring to the "Vulnerable! ... version" output? The reason I changed this is because it makes it easier to double check afterwards if this is a false positive. Especially when you cannot run further commands (I only got the output of lse for review...). But you are right. Maybe I should check whether the variable is empty and use the command version then?
Yes I meant that part and yes, I think it is better to be sure that the variable is not empty before using it.
Done :)
Sorry for the delay on this and thank you for addressing this. Merging now.
Fixed several false positives for RHEL.
Unfortunately I'm not able to test the changes now, because I do not have access to a RHEL system. But I got the output of lse from a run on a RHEL system and it was full of false positives, so I needed to fix this up :D