diff mbox series

[2/2] libsepol: avoid fixed sized format buffer for xperms

Message ID 20231120143145.28831-2-cgzones@googlemail.com (mailing list archive)
State Accepted
Commit 285d7cc81bb1
Delegated to: Petr Lautrbach
Headers show
Series [1/2] libsepol: avoid fixed sized format buffer for xperms | expand

Commit Message

Christian Göttsche Nov. 20, 2023, 2:31 p.m. UTC
An extended access vector rule can consist of many individual ranges of
permissions.  Use a dynamically growing sized buffer for formatting such
rules instead of a static buffer to avoid write failures due to
truncations.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 checkpolicy/test/dismod.c     |  9 +++++-
 checkpolicy/test/dispol.c     | 10 ++++++-
 libsepol/src/assertion.c      |  7 ++++-
 libsepol/src/kernel_to_conf.c |  9 +++---
 libsepol/src/util.c           | 54 +++++++++++++++++++++++------------
 5 files changed, 64 insertions(+), 25 deletions(-)
diff mbox series

Patch

diff --git a/checkpolicy/test/dismod.c b/checkpolicy/test/dismod.c
index fa7117f5..9f4a669b 100644
--- a/checkpolicy/test/dismod.c
+++ b/checkpolicy/test/dismod.c
@@ -347,6 +347,7 @@  static int display_avrule(avrule_t * avrule, policydb_t * policy,
 		display_id(policy, fp, SYM_TYPES, avrule->perms->data - 1, "");
 	} else if (avrule->specified & AVRULE_XPERMS) {
 		avtab_extended_perms_t xperms;
+		char *perms;
 		int i;
 
 		if (avrule->xperms->specified == AVRULE_XPERMS_IOCTLFUNCTION)
@@ -362,7 +363,13 @@  static int display_avrule(avrule_t * avrule, policydb_t * policy,
 		for (i = 0; i < EXTENDED_PERMS_LEN; i++)
 			xperms.perms[i] = avrule->xperms->perms[i];
 
-		fprintf(fp, "%s", sepol_extended_perms_to_string(&xperms));
+		perms = sepol_extended_perms_to_string(&xperms);
+		if (!perms) {
+			fprintf(fp, "     ERROR: failed to format xperms\n");
+			return -1;
+		}
+		fprintf(fp, "%s", perms);
+		free(perms);
 	}
 
 	fprintf(fp, ";\n");
diff --git a/checkpolicy/test/dispol.c b/checkpolicy/test/dispol.c
index b567ce77..944ef7ec 100644
--- a/checkpolicy/test/dispol.c
+++ b/checkpolicy/test/dispol.c
@@ -196,6 +196,8 @@  static int render_av_rule(avtab_key_t * key, avtab_datum_t * datum, uint32_t wha
 			fprintf(fp, ";\n");
 		}
 	} else if (key->specified & AVTAB_XPERMS) {
+		char *perms;
+
 		if (key->specified & AVTAB_XPERMS_ALLOWED)
 			fprintf(fp, "allowxperm ");
 		else if (key->specified & AVTAB_XPERMS_AUDITALLOW)
@@ -203,7 +205,13 @@  static int render_av_rule(avtab_key_t * key, avtab_datum_t * datum, uint32_t wha
 		else if (key->specified & AVTAB_XPERMS_DONTAUDIT)
 			fprintf(fp, "dontauditxperm ");
 		render_key(key, p, fp);
-		fprintf(fp, "%s;\n", sepol_extended_perms_to_string(datum->xperms));
+		perms = sepol_extended_perms_to_string(datum->xperms);
+		if (!perms) {
+			fprintf(fp, "     ERROR: failed to format xperms\n");
+			return -1;
+		}
+		fprintf(fp, "%s;\n", perms);
+		free(perms);
 	} else {
 		fprintf(fp, "     ERROR: no valid rule type specified\n");
 		return -1;
diff --git a/libsepol/src/assertion.c b/libsepol/src/assertion.c
index b6ac4cfe..6de7d031 100644
--- a/libsepol/src/assertion.c
+++ b/libsepol/src/assertion.c
@@ -178,15 +178,20 @@  static int report_assertion_extended_permissions(sepol_handle_t *handle,
 				rc = check_extended_permissions(avrule->xperms, xperms);
 				/* failure on the extended permission check_extended_permissions */
 				if (rc) {
+					char *permstring;
+
 					extended_permissions_violated(&error, avrule->xperms, xperms);
+					permstring = sepol_extended_perms_to_string(&error);
+
 					ERR(handle, "neverallowxperm on line %lu of %s (or line %lu of %s) violated by\n"
 							"allowxperm %s %s:%s %s;",
 							avrule->source_line, avrule->source_filename, avrule->line, policy_name(p),
 							p->p_type_val_to_name[i],
 							p->p_type_val_to_name[j],
 							p->p_class_val_to_name[curperm->tclass - 1],
-							sepol_extended_perms_to_string(&error));
+							permstring ?: "<format-failure>");
 
+					free(permstring);
 					errors++;
 				}
 			}
diff --git a/libsepol/src/kernel_to_conf.c b/libsepol/src/kernel_to_conf.c
index b0ae16d9..b5b530d6 100644
--- a/libsepol/src/kernel_to_conf.c
+++ b/libsepol/src/kernel_to_conf.c
@@ -1683,7 +1683,7 @@  static char *avtab_node_to_str(struct policydb *pdb, avtab_key_t *key, avtab_dat
 	uint32_t data = datum->data;
 	type_datum_t *type;
 	const char *flavor, *src, *tgt, *class, *perms, *new;
-	char *rule = NULL;
+	char *rule = NULL, *permstring;
 
 	switch (0xFFF & key->specified) {
 	case AVTAB_ALLOWED:
@@ -1738,13 +1738,14 @@  static char *avtab_node_to_str(struct policydb *pdb, avtab_key_t *key, avtab_dat
 		rule = create_str("%s %s %s:%s { %s };", 5,
 				  flavor, src, tgt, class, perms+1);
 	} else if (key->specified & AVTAB_XPERMS) {
-		perms = sepol_extended_perms_to_string(datum->xperms);
-		if (perms == NULL) {
+		permstring = sepol_extended_perms_to_string(datum->xperms);
+		if (permstring == NULL) {
 			ERR(NULL, "Failed to generate extended permission string");
 			goto exit;
 		}
 
-		rule = create_str("%s %s %s:%s %s;", 5, flavor, src, tgt, class, perms);
+		rule = create_str("%s %s %s:%s %s;", 5, flavor, src, tgt, class, permstring);
+		free(permstring);
 	} else {
 		new = pdb->p_type_val_to_name[data - 1];
 
diff --git a/libsepol/src/util.c b/libsepol/src/util.c
index 0a2edc85..25a3ed55 100644
--- a/libsepol/src/util.c
+++ b/libsepol/src/util.c
@@ -133,19 +133,29 @@  char *sepol_extended_perms_to_string(avtab_extended_perms_t *xperms)
 	uint16_t low_value;
 	unsigned int bit;
 	unsigned int in_range = 0;
-	static char xpermsbuf[2048];
-	char *p;
-	int len, xpermslen = 0;
-	xpermsbuf[0] = '\0';
-	p = xpermsbuf;
+	char *buffer = NULL, *p;
+	int len;
+	size_t remaining, size = 128;
 
 	if ((xperms->specified != AVTAB_XPERMS_IOCTLFUNCTION)
 		&& (xperms->specified != AVTAB_XPERMS_IOCTLDRIVER))
 		return NULL;
 
-	len = snprintf(p, sizeof(xpermsbuf) - xpermslen, "ioctl { ");
+retry:
+	size *= 2;
+	if (size == 0)
+		goto err;
+	p = realloc(buffer, size);
+	if (!p)
+		goto err;
+	buffer = p;
+	remaining = size;
+
+	len = snprintf(p, remaining, "ioctl { ");
+	if (len < 0 || (size_t)len >= remaining)
+		goto err;
 	p += len;
-	xpermslen += len;
+	remaining -= len;
 
 	for (bit = 0; bit < sizeof(xperms->perms)*8; bit++) {
 		if (!xperm_test(bit, xperms->perms))
@@ -165,35 +175,43 @@  char *sepol_extended_perms_to_string(avtab_extended_perms_t *xperms)
 			value = xperms->driver<<8 | bit;
 			if (in_range) {
 				low_value = xperms->driver<<8 | low_bit;
-				len = snprintf(p, sizeof(xpermsbuf) - xpermslen, "0x%hx-0x%hx ", low_value, value);
+				len = snprintf(p, remaining, "0x%hx-0x%hx ", low_value, value);
 			} else {
-				len = snprintf(p, sizeof(xpermsbuf) - xpermslen, "0x%hx ", value);
+				len = snprintf(p, remaining, "0x%hx ", value);
 			}
 		} else if (xperms->specified & AVTAB_XPERMS_IOCTLDRIVER) {
 			value = bit << 8;
 			if (in_range) {
 				low_value = low_bit << 8;
-				len = snprintf(p, sizeof(xpermsbuf) - xpermslen, "0x%hx-0x%hx ", low_value, (uint16_t) (value|0xff));
+				len = snprintf(p, remaining, "0x%hx-0x%hx ", low_value, (uint16_t) (value|0xff));
 			} else {
-				len = snprintf(p, sizeof(xpermsbuf) - xpermslen, "0x%hx-0x%hx ", value, (uint16_t) (value|0xff));
+				len = snprintf(p, remaining, "0x%hx-0x%hx ", value, (uint16_t) (value|0xff));
 			}
 
 		}
 
-		if (len < 0 || (size_t) len >= (sizeof(xpermsbuf) - xpermslen))
-			return NULL;
+		if (len < 0)
+			goto err;
+		if ((size_t) len >= remaining)
+			goto retry;
 
 		p += len;
-		xpermslen += len;
+		remaining -= len;
 		if (in_range)
 			in_range = 0;
 	}
 
-	len = snprintf(p, sizeof(xpermsbuf) - xpermslen, "}");
-	if (len < 0 || (size_t) len >= (sizeof(xpermsbuf) - xpermslen))
-		return NULL;
+	len = snprintf(p, remaining, "}");
+	if (len < 0)
+		goto err;
+	if ((size_t) len >= remaining)
+		goto retry;
+
+	return buffer;
 
-	return xpermsbuf;
+err:
+	free(buffer);
+	return NULL;
 }
 
 /*