Review OpenScanHub results for libdnf

[ppisar]

Fedora scanned F41 critical packages for insecure pieces of source code https://lists.fedoraproject.org/archives/list/[email protected]/thread/ZNWA2K3H6OS3LFJOTA5H4FJJC64EBLRK/ Results are at https://svashisht.fedorapeople.org/f41-22-Apr-2024/. There are some findings for libdnf. It would be great to review and the address true positives.

[siteshwar]

A more recent report is available at https://svashisht.fedorapeople.org/f41-03-Jul-2024/

But it contains high number of false positives due to cppcheck warning about limiting analysis of branches. It should be fixed in the future mass scans.

[siteshwar]

Report for Fedora 42 (rawhide) is available at https://svashisht.fedorapeople.org/openscanhub/mass-scans/f42-13-Nov-2024/, it significantly reduces number of false positives.

[kontura]

Most of the warnings come from code generated by swig. While there is a chance we might be able to fix some of those on libdnf side I have already tried to do something similar in the past and didn't succeed. I am not sure if it is worth to invest more time into it.

Then there are several glib2 deprecations:

libdnf-0.73.3-build/libdnf-0.73.3/libdnf/dnf-repo-loader.cpp:248:31: warning[-Wdeprecated-declarations]: ‘GList* g_unix_mounts_get(guint64*)’ is deprecated: Use 'g_unix_mount_entries_get' instead
libdnf-0.73.3-build/libdnf-0.73.3/libdnf/dnf-repo-loader.cpp:251:38: warning[-Wdeprecated-declarations]: ‘gboolean g_unix_mount_is_readonly(GUnixMountEntry*)’ is deprecated: Use 'g_unix_mount_entry_is_readonly' instead
libdnf-0.73.3-build/libdnf-0.73.3/libdnf/dnf-repo-loader.cpp:253:47: warning[-Wdeprecated-declarations]: ‘const char* g_unix_mount_get_fs_type(GUnixMountEntry*)’ is deprecated: Use 'g_unix_mount_entry_get_fs_type' instead
libdnf-0.73.3-build/libdnf-0.73.3/libdnf/dnf-repo-loader.cpp:256:78: warning[-Wdeprecated-declarations]: ‘const char* g_unix_mount_get_mount_path(GUnixMountEntry*)’ is deprecated: Use 'g_unix_mount_entry_get_mount_path' instead
libdnf-0.73.3-build/libdnf-0.73.3/libdnf/dnf-repo-loader.cpp:263:35: warning[-Wdeprecated-declarations]: ‘void g_unix_mount_free(GUnixMountEntry*)’ is deprecated: Use 'g_unix_mount_entry_free' instead

However these are quite new and the recommended functions are available only since 2.84 which is present in >= Fedora42.
I think we should wait with these.

Next there is a false positive:

libdnf-0.73.3-build/libdnf-0.73.3/libdnf/hy-iutil.cpp:548: error[returnDanglingLifetime]: Returning pointer to local variable 'dup' that will be invalid when returning.

I believe this is also a failse positive:

libdnf-0.73.3-build/libdnf-0.73.3/libdnf/hy-package.cpp:58:3: warning[-Wodr]: type ‘struct DnfPackagePrivate’ violates the C++ One Definition Rule

While the class type DnfPackagePrivate does appear twice with different content it is in separate translation units.

Issue:

libdnf-0.73.3-build/libdnf-0.73.3/libdnf/utils/sqlite3/Sqlite3.hpp:337: error[throwInNoexceptFunction]: Exception thrown in function declared not to throw exceptions.

is already tracked as #1686

Finally for:

libdnf-0.73.3-build/libdnf-0.73.3/tests/hawkey/test_goal.cpp:54:17: warning[-Wformat-extra-args]: too many arguments for format
libdnf-0.73.3-build/libdnf-0.73.3/tests/hawkey/test_goal.cpp:439:18: warning[-Wformat-extra-args]: too many arguments for format
libdnf-0.73.3-build/libdnf-0.73.3/tests/hawkey/test_goal.cpp:449:32: warning[-Wformat-extra-args]: too many arguments for format
libdnf-0.73.3-build/libdnf-0.73.3/tests/hawkey/test_goal.cpp:458:33: warning[-Wformat-extra-args]: too many arguments for format

I made a PR.

[siteshwar]

@kontura Thanks for the review! Meanwhile, a new mass scan was performed last month and results can be seen here. However, the results look pretty much the same, so further review may not be needed.

[siteshwar]

@kontura Shall I add path under libdnf-0.73.3-build/libdnf-0.73.3/build-py3 to known-false-positives excluded paths?

[kontura]

@kontura Thanks for the review! Meanwhile, a new mass scan was performed last month and results can be seen here. However, the results look pretty much the same, so further review may not be needed.

Thanks, I looked at it and added two new commits to my PR based on two new reports.

@kontura Shall I add path under libdnf-0.73.3-build/libdnf-0.73.3/build-py3 to known-false-positives excluded paths?

I am not sure, I don't know what is the purpose of these reports. If they are only for us developers I probably wouldn't add it. I am not certain the SWIG issues are false positives and I don't mind them.

[siteshwar]

@kontura Thanks for the review! Meanwhile, a new mass scan was performed last month and results can be seen here. However, the results look pretty much the same, so further review may not be needed.

Thanks, I looked at it and added two new commits to my PR based on two new reports.

@kontura Shall I add path under libdnf-0.73.3-build/libdnf-0.73.3/build-py3 to known-false-positives excluded paths?

I am not sure, I don't know what is the purpose of these reports. If they are only for us developers I probably wouldn't add it. I am not certain the SWIG issues are false positives and I don't mind them.

They are meant for upstream developers and package maintainers. I would avoid adding build-py3 directory to excluded paths for now.