db: consider control characters in filename as malicious

Especially a newline can produce havoc in the database file as
the filename is written there as-is. This hardenes the extraction
to consider any control character as malicious. Additional
hardening is added to database loading to better detect corrupt
state and return proper error code about it.

Reported-by: Luca Weiss <luca@z3ntu.xyz>

3 files changed, 22 insertions, 11 deletions

diff --git a/src/apk_defines.h b/src/apk_defines.h
index 1bc8db0..92e97d8 100644
--- a/src/apk_defines.h
+++ b/src/apk_defines.h
@@ -36,6 +36,7 @@
 #define EAPKSTALEINDEX		1025
 #define EAPKFORMAT		1026
 #define EAPKDEPFORMAT		1027
+#define EAPKDBFORMAT		1028
 
 static inline void *ERR_PTR(long error) { return (void*) error; }
 static inline void *ERR_CAST(const void *ptr) { return (void*) ptr; }
diff --git a/src/database.c b/src/database.c
index b6d315b..1e506c1 100644
--- a/src/database.c
+++ b/src/database.c
@@ -505,6 +505,8 @@ struct apk_package *apk_db_pkg_add(struct apk_database *db, struct apk_package *
 	struct apk_package *idb;
 	struct apk_dependency *dep;
 
+	if (!pkg->name || !pkg->version) return NULL;
+
 	if (!pkg->license) pkg->license = &apk_atom_null;
 
 	/* Set as "cached" if installing from specified file, and
@@ -765,7 +767,7 @@ int apk_db_index_read(struct apk_database *db, struct apk_istream *is, int repo)
 	while (!APK_BLOB_IS_NULL(l = apk_istream_get_delim(is, token))) {
 		lineno++;
 
-		if (l.len < 2 || l.ptr[1] != ':') {
+		if (l.len < 2) {
 			if (pkg == NULL)
 				continue;
 
@@ -780,10 +782,8 @@ int apk_db_index_read(struct apk_database *db, struct apk_istream *is, int repo)
 				ipkg = apk_pkg_install(db, pkg);
 			}
 
-			if (apk_db_pkg_add(db, pkg) == NULL) {
-				apk_err(out, "Installed database load failed");
-				return -1;
-			}
+			if (apk_db_pkg_add(db, pkg) == NULL)
+				goto err_fmt;
 			pkg = NULL;
 			ipkg = NULL;
 			continue;
@@ -791,6 +791,7 @@ int apk_db_index_read(struct apk_database *db, struct apk_istream *is, int repo)
 
 		/* Get field */
 		field = l.ptr[0];
+		if (l.ptr[1] != ':') goto err_fmt;
 		l.ptr += 2;
 		l.len -= 2;
 
@@ -888,12 +889,11 @@ int apk_db_index_read(struct apk_database *db, struct apk_istream *is, int repo)
 old_apk_tools:
 	/* Installed db should not have unsupported fields */
 	apk_err(out, "This apk-tools is too old to handle installed packages");
-	is->err = -EAPKFORMAT;
-	goto err;
+	goto err_fmt;
 bad_entry:
 	apk_err(out, "FDB format error (line %d, entry '%c')", lineno, field);
-	is->err = -EAPKFORMAT;
-err:
+err_fmt:
+	is->err = -EAPKDBFORMAT;
 	return apk_istream_close(is);
 }
 
@@ -1709,7 +1709,7 @@ ret_errno:
 	r = -errno;
 ret_r:
 	if (msg != NULL)
-		apk_err(out, "%s: %s", msg, strerror(-r));
+		apk_err(out, "%s: %s", msg, apk_error_str(-r));
 	apk_db_close(db);
 
 	return r;
@@ -2356,6 +2356,14 @@ static const char *format_tmpname(struct apk_package *pkg, struct apk_db_file *f
 	return tmpname;
 }
 
+static int contains_control_character(const char *str)
+{
+	for (; *str; str++) {
+		if (*str < 0x20 || *str == 0x7f) return 1;
+	}
+	return 0;
+}
+
 static int apk_db_install_archive_entry(void *_ctx,
 					const struct apk_file_info *ae,
 					struct apk_istream *is)
@@ -2403,7 +2411,7 @@ static int apk_db_install_archive_entry(void *_ctx,
 		return 0;
 
 	/* Sanity check the file name */
-	if (ae->name[0] == '/' ||
+	if (ae->name[0] == '/' || contains_control_character(ae->name) ||
 	    strncmp(ae->name, &dot1[1], 2) == 0 ||
 	    strncmp(ae->name, &dot2[1], 3) == 0 ||
 	    strstr(ae->name, dot1) || strstr(ae->name, dot2)) {
diff --git a/src/print.c b/src/print.c
index 829f7f5..e9e1dfb 100644
--- a/src/print.c
+++ b/src/print.c
@@ -58,6 +58,8 @@ const char *apk_error_str(int error)
 		return "package file format error";
 	case EAPKDEPFORMAT:
 		return "package dependency format error";
+	case EAPKDBFORMAT:
+		return "database file format error";
 	default:
 		return strerror(error);
 	}