CVE-2023-46139: Incorrect Authorization (CWE-863)
value: KernelSU is a Kernel based root solution for Android. Starting in version 0.6.1 and prior to version 0.7.0, if a KernelSU installed device is infected with a malware whose app signing block specially constructed, it can take over root privileges on the device. The vulnerable verification logic actually obtains the signature of the last block with an id of `0x7109871a`, while the verification logic during Android installation is to obtain the first one. In addition to the actual signature upgrade that has been fixed (KSU thought it was V2 but was actually V3), there is also the problem of actual signature downgrading (KSU thought it was V2 but was actually V1). Find a condition in the signature verification logic that will cause the signature not to be found error, and KernelSU does not implement the same conditions, so KSU thinks there is a V2 signature, but the APK signature verification actually uses the V1 signature. This issue is fixed in version 0.7.0. As workarounds, keep the KernelSU manager installed and avoid installing unknown apps.
CWE description: The product performs an authorization check when an actor attempts to access a resource or perform an action, but it does not correctly perform the check. This allows attackers to bypass intended access restrictions.
CVSS v3 base score: 5.7
Published: 2023-10-31T00:15Z
Repository: https://github.com/tiann/KernelSU
Language: C
Files changed: kernel/Makefile, kernel/apk_sign.c, kernel/manager.c
Commit date: 2023-10-23 12:59:30 +0800
Commit message: Merge pull request from GHSA-86cp-3prf-pwqq

* kernel: deny v2 signature blocks with incorrect number

* kernel: reject v1 signature

* kernel: enforce manager package name at compile time

* kernel: don't specific package name in source code, use it in ci
Diff: diff --git a/kernel/Makefile b/kernel/Makefile
index b58786e..be90948 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -30,10 +30,15 @@ endif
 
 ifndef KSU_EXPECTED_HASH
 KSU_EXPECTED_HASH := c371061b19d8c7d7d6133c6a9bafe198fa944e50c1b31c9d8daa8d7f1fc2d2d6
 endif
 
+ifdef KSU_MANAGER_PACKAGE
+ccflags-y += -DKSU_MANAGER_PACKAGE=\"$(KSU_MANAGER_PACKAGE)\"
+$(info -- KernelSU Manager package name: $(KSU_MANAGER_PACKAGE))
+endif
+
 $(info -- KernelSU Manager signature size: $(KSU_EXPECTED_SIZE))
 $(info -- KernelSU Manager signature hash: $(KSU_EXPECTED_HASH))
 
 ccflags-y += -DEXPECTED_SIZE=$(KSU_EXPECTED_SIZE)
 ccflags-y += -DEXPECTED_HASH=\"$(KSU_EXPECTED_HASH)\"
diff --git a/kernel/apk_sign.c b/kernel/apk_sign.c
index 5f7175c..fda32ac 100644
--- a/kernel/apk_sign.c
+++ b/kernel/apk_sign.c
@@ -51,11 +51,11 @@ static int calc_hash(struct crypto_shash *alg, const unsigned char *data,
 	kfree(sdesc);
 	return ret;
 }
 
 static int ksu_sha256(const unsigned char *data, unsigned int datalen,
-		unsigned char *digest)
+		      unsigned char *digest)
 {
 	struct crypto_shash *alg;
 	char *hash_alg_name = "sha256";
 	int ret;
 
@@ -68,11 +68,11 @@ static int ksu_sha256(const unsigned char *data, unsigned int datalen,
 	crypto_free_shash(alg);
 	return ret;
 }
 
 static bool check_block(struct file *fp, u32 *size4, loff_t *pos, u32 *offset,
-			unsigned expected_size, const char* expected_sha256)
+			unsigned expected_size, const char *expected_sha256)
 {
 	ksu_kernel_read_compat(fp, size4, 0x4, pos); // signer-sequence length
 	ksu_kernel_read_compat(fp, size4, 0x4, pos); // signer length
 	ksu_kernel_read_compat(fp, size4, 0x4, pos); // signed data length
 
@@ -88,11 +88,11 @@ static bool check_block(struct file *fp, u32 *size4, loff_t *pos, u32 *offset,
 	*offset += 0x4 * 2;
 
 	if (*size4 == expected_size) {
 		*offset += *size4;
 
-		#define CERT_MAX_LENGTH 1024
+#define CERT_MAX_LENGTH 1024
 		char cert[CERT_MAX_LENGTH];
 		if (*size4 > CERT_MAX_LENGTH) {
 			pr_info("cert length overlimit\n");
 			return false;
 		}
@@ -105,28 +105,84 @@ static bool check_block(struct file *fp, u32 *size4, loff_t *pos, u32 *offset,
 
 		char hash_str[SHA256_DIGEST_SIZE * 2 + 1];
 		hash_str[SHA256_DIGEST_SIZE * 2] = '\0';
 
 		bin2hex(hash_str, digest, SHA256_DIGEST_SIZE);
-		pr_info("sha256: %s, expected: %s\n", hash_str, expected_sha256);
+		pr_info("sha256: %s, expected: %s\n", hash_str,
+			expected_sha256);
 		if (strcmp(expected_sha256, hash_str) == 0) {
 			return true;
 		}
 	}
 	return false;
 }
 
-static __always_inline bool
-check_v2_signature(char *path, unsigned expected_size, const char *expected_sha256)
+struct zip_entry_header {
+	uint32_t signature;
+	uint16_t version;
+	uint16_t flags;
+	uint16_t compression;
+	uint16_t mod_time;
+	uint16_t mod_date;
+	uint32_t crc32;
+	uint32_t compressed_size;
+	uint32_t uncompressed_size;
+	uint16_t file_name_length;
+	uint16_t extra_field_length;
+} __attribute__((packed));
+
+// This is a necessary but not sufficient condition, but it is enough for us
+static bool has_v1_signature_file(struct file *fp)
+{
+	struct zip_entry_header header;
+	const char MANIFEST[] = "META-INF/MANIFEST.MF";
+
+	loff_t pos = 0;
+
+	while (ksu_kernel_read_compat(fp, &header,
+				      sizeof(struct zip_entry_header), &pos) ==
+	       sizeof(struct zip_entry_header)) {
+		if (header.signature != 0x04034b50) {
+			// ZIP magic: 'PK'
+			return false;
+		}
+		// Read the entry file name
+		if (header.file_name_length == sizeof(MANIFEST) - 1) {
+			char fileName[sizeof(MANIFEST)];
+			ksu_kernel_read_compat(fp, fileName,
+					       header.file_name_length, &pos);
+			fileName[header.file_name_length] = '\0';
+
+			// Check if the entry matches META-INF/MANIFEST.MF
+			if (strncmp(MANIFEST, fileName, sizeof(MANIFEST) - 1) ==
+			    0) {
+				return true;
+			}
+		} else {
+			// Skip the entry file name
+			pos += header.file_name_length;
+		}
+
+		// Skip to the next entry
+		pos += header.extra_field_length + header.compressed_size;
+	}
+
+	return false;
+}
+
+static __always_inline bool check_v2_signature(char *path,
+					       unsigned expected_size,
+					       const char *expected_sha256)
 {
 	unsigned char buffer[0x11] = { 0 };
 	u32 size4;
 	u64 size8, size_of_block;
 
 	loff_t pos;
 
 	bool v2_signing_valid = false;
+	int v2_signing_blocks = 0;
 	bool v3_signing_exist = false;
 	bool v3_1_signing_exist = false;
 
 	int i;
 	struct file *fp = ksu_filp_open_compat(path, O_RDONLY, 0);
@@ -183,22 +239,38 @@ check_v2_signature(char *path, unsigned expected_size, const char *expected_sha2
 		}
 		ksu_kernel_read_compat(fp, &id, 0x4, &pos); // id
 		offset = 4;
 		pr_info("id: 0x%08x\n", id);
 		if (id == 0x7109871au) {
-			v2_signing_valid = check_block(fp, &size4, &pos, &offset,
-						  expected_size, expected_sha256);
+			v2_signing_blocks++;
+			v2_signing_valid =
+				check_block(fp, &size4, &pos, &offset,
+					    expected_size, expected_sha256);
 		} else if (id == 0xf05368c0u) {
 			// http://aospxref.com/android-14.0.0_r2/xref/frameworks/base/core/java/android/util/apk/ApkSignatureSchemeV3Verifier.java#73
 			v3_signing_exist = true;
 		} else if (id == 0x1b93ad61u) {
 			// http://aospxref.com/android-14.0.0_r2/xref/frameworks/base/core/java/android/util/apk/ApkSignatureSchemeV3Verifier.java#74
 			v3_1_signing_exist = true;
 		}
 		pos += (size8 - offset);
 	}
 
+	if (v2_signing_blocks != 1) {
+		pr_err("Unexpected v2 signature count: %d\n",
+		       v2_signing_blocks);
+		v2_signing_valid = false;
+	}
+
+	if (v2_signing_valid) {
+		int has_v1_signing = has_v1_signature_file(fp);
+		if (has_v1_signing) {
+			pr_err("Unexpected v1 signature scheme found!\n");
+			filp_close(fp, 0);
+			return false;
+		}
+	}
 clean:
 	filp_close(fp, 0);
 
 	if (v3_signing_exist || v3_1_signing_exist) {
 		pr_err("Unexpected v3 signature scheme found!\n");
diff --git a/kernel/manager.c b/kernel/manager.c
index 83a2226..fcb1b18 100644
--- a/kernel/manager.c
+++ b/kernel/manager.c
@@ -22,10 +22,19 @@ bool become_manager(char *pkg)
 	struct path files_path;
 	char *cwd;
 	char *buf;
 	bool result = false;
 
+#ifdef KSU_MANAGER_PACKAGE
+	// pkg is `/<real package>`
+	if (strncmp(pkg + 1, KSU_MANAGER_PACKAGE,
+		    sizeof(KSU_MANAGER_PACKAGE) - 1) != 0) {
+		pr_info("manager package is inconsistent with kernel build: %s\n",
+			KSU_MANAGER_PACKAGE);
+		return false;
+	}
+#endif
 	// must be zygote's direct child, otherwise any app can fork a new process and
 	// open manager's apk
 	if (task_uid(current->real_parent).val != 0) {
 		pr_info("parent is not zygote!\n");
 		return false;

Vulnerable code: 		unsigned char *digest)
			unsigned expected_size, const char* expected_sha256)
		#define CERT_MAX_LENGTH 1024
		pr_info("sha256: %s, expected: %s\n", hash_str, expected_sha256);
static __always_inline bool
check_v2_signature(char *path, unsigned expected_size, const char *expected_sha256)
			v2_signing_valid = check_block(fp, &size4, &pos, &offset,
						  expected_size, expected_sha256);
Fixed code: ifdef KSU_MANAGER_PACKAGE
ccflags-y += -DKSU_MANAGER_PACKAGE=\"$(KSU_MANAGER_PACKAGE)\"
$(info -- KernelSU Manager package name: $(KSU_MANAGER_PACKAGE))
endif

		      unsigned char *digest)
			unsigned expected_size, const char *expected_sha256)
#define CERT_MAX_LENGTH 1024
		pr_info("sha256: %s, expected: %s\n", hash_str,
			expected_sha256);
struct zip_entry_header {
	uint32_t signature;
	uint16_t version;
	uint16_t flags;
	uint16_t compression;
	uint16_t mod_time;
	uint16_t mod_date;
	uint32_t crc32;
	uint32_t compressed_size;
	uint32_t uncompressed_size;
	uint16_t file_name_length;
	uint16_t extra_field_length;
} __attribute__((packed));

// This is a necessary but not sufficient condition, but it is enough for us
static bool has_v1_signature_file(struct file *fp)
{
	struct zip_entry_header header;
	const char MANIFEST[] = "META-INF/MANIFEST.MF";

	loff_t pos = 0;

	while (ksu_kernel_read_compat(fp, &header,
				      sizeof(struct zip_entry_header), &pos) ==
	       sizeof(struct zip_entry_header)) {
		if (header.signature != 0x04034b50) {
			// ZIP magic: 'PK'
			return false;
		}
		// Read the entry file name
		if (header.file_name_length == sizeof(MANIFEST) - 1) {
			char fileName[sizeof(MANIFEST)];
			ksu_kernel_read_compat(fp, fileName,
					       header.file_name_length, &pos);
			fileName[header.file_name_length] = '\0';

			// Check if the entry matches META-INF/MANIFEST.MF
			if (strncmp(MANIFEST, fileName, sizeof(MANIFEST) - 1) ==
			    0) {
				return true;
			}
		} else {
			// Skip the entry file name
			pos += header.file_name_length;
		}

		// Skip to the next entry
		pos += header.extra_field_length + header.compressed_size;
	}

	return false;
}

static __always_inline bool check_v2_signature(char *path,
					       unsigned expected_size,
					       const char *expected_sha256)
	int v2_signing_blocks = 0;
			v2_signing_blocks++;
			v2_signing_valid =
				check_block(fp, &size4, &pos, &offset,
					    expected_size, expected_sha256);
	if (v2_signing_blocks != 1) {
		pr_err("Unexpected v2 signature count: %d\n",
		       v2_signing_blocks);
		v2_signing_valid = false;
	}

	if (v2_signing_valid) {
		int has_v1_signing = has_v1_signature_file(fp);
		if (has_v1_signing) {
			pr_err("Unexpected v1 signature scheme found!\n");
			filp_close(fp, 0);
			return false;
		}
	}
#ifdef KSU_MANAGER_PACKAGE
	// pkg is `/<real package>`
	if (strncmp(pkg + 1, KSU_MANAGER_PACKAGE,
		    sizeof(KSU_MANAGER_PACKAGE) - 1) != 0) {
		pr_info("manager package is inconsistent with kernel build: %s\n",
			KSU_MANAGER_PACKAGE);
		return false;
	}
#endif