diff mbox series

[opkg,v2] libopkg: harden checksum verification in error cases

Message ID 20200824130959.437942-1-baptiste@bitsofnetworks.org
State Needs Review / ACK
Headers show
Series [opkg,v2] libopkg: harden checksum verification in error cases | expand

Commit Message

Baptiste Jonglez Aug. 24, 2020, 1:09 p.m. UTC
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.

Signed-off-by: Baptiste Jonglez <git@bitsofnetworks.org>
---

v2: rebase to avoid depending on the other patch "libopkg: move file size
check after checksum verification".

 libopkg/opkg_install.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Baptiste Jonglez Aug. 24, 2020, 5:26 p.m. UTC | #1
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 mbox series

Patch

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);