| Message ID | 20200824130959.437942-1-baptiste@bitsofnetworks.org |
|---|---|
| State | Under Review |
| Delegated to: | Baptiste Jonglez |
| Headers | show |
| Series | [opkg,v2] libopkg: harden checksum verification in error cases | expand |
On 24-08-20, Baptiste Jonglez wrote: > From: Baptiste Jonglez <git@bitsofnetworks.org> > > This should make it harder to exploit bugs such as CVE-2020-7982. > > If we can't compute the checksum of a package, we should abort. > > Similarly, if we can't find any checksum in the package index, this should > yield an error. > > As an exception, installing a package directly from a file is allowed even > if no checksum is found, because this is typically used without any > package index. This can be useful when installing packages "manually" on > a device, but is also done in several places during the OpenWrt build > process. > > In any case, it is always possible to use the existing --force-checksum > option to manually bypass these new verifications. It seems that I missed a use-case: installing a package directly from an URL, like this: opkg install http://example.com/pkg.ipk It will now fail because no checksum is found in a package index. One way would be to also enable the "provided_by_hand" flag in this case, just like it is already done when installing from a file (e.g. opkg install /tmp/foo.ipk) It seems this could change dependency resolution, that's apparently the purpose of the "provided_by_hand" flag according to a comment: Adding this flag, to "force" opkg to choose a "provided_by_hand" package, if there are multiple choice Is it fine? Any other idea? Baptiste
diff --git a/libopkg/opkg_install.c b/libopkg/opkg_install.c index 27c9484..18a2511 100644 --- a/libopkg/opkg_install.c +++ b/libopkg/opkg_install.c @@ -1395,6 +1395,11 @@ int opkg_install_pkg(pkg_t * pkg, int from_upgrade) pkg_md5 = pkg_get_md5(pkg); if (pkg_md5) { file_md5 = file_md5sum_alloc(local_filename); + if (!file_md5 && !conf->force_checksum) { + opkg_msg(ERROR, "Failed to compute md5sum of package %s.\n", + pkg->name); + return -1; + } if (file_md5 && strcmp(file_md5, pkg_md5)) { if (!conf->force_checksum) { opkg_msg(ERROR, "Package %s md5sum mismatch. " @@ -1416,6 +1421,11 @@ int opkg_install_pkg(pkg_t * pkg, int from_upgrade) pkg_sha256 = pkg_get_sha256(pkg); if (pkg_sha256) { file_sha256 = file_sha256sum_alloc(local_filename); + if (!file_sha256 && !conf->force_checksum) { + opkg_msg(ERROR, "Failed to compute sha256sum of package %s.\n", + pkg->name); + return -1; + } if (file_sha256 && strcmp(file_sha256, pkg_sha256)) { if (!conf->force_checksum) { opkg_msg(ERROR, @@ -1434,6 +1444,16 @@ int opkg_install_pkg(pkg_t * pkg, int from_upgrade) free(file_sha256); } + /* Check that at least one type of checksum was found. There are + * two acceptable exceptions: + * 1) the package is explicitly installed from a local file; + * 2) the --force-checksum option is used to disable checksum verification. */ + if (!pkg_md5 && !pkg_sha256 && !pkg->provided_by_hand && !conf->force_checksum) { + opkg_msg(ERROR, "Failed to obtain checksum of package %s from package index.\n", + pkg->name); + return -1; + } + if (conf->download_only) { if (conf->nodeps == 0) { err = satisfy_dependencies_for(pkg);