[wireless-regdb] [PATCH] reglib: Validate all structure and array lengths

Ben Hutchings ben at decadent.org.uk
Sun Jun 30 19:49:31 EDT 2013


Add checks that:
- Signature length does not exceed the file length (this was already
  checked, but did not account for signature lengths greater than 2 GB)
- Database length is long enough for all structures we expect in it
- Array length calculations will not overflow

To keep these checks simple, change the types of array length and index
variables to unsigned int (must be at least 32-bit, matching the file
format) and the types of byte-length variables to size_t.

Alexandre Rebert <alexandre at cmu.edu> reported and provided a test case
for the signature length issue; the others I found by inspection.

Signed-off-by: Ben Hutchings <ben at decadent.org.uk>
---
 reglib.c | 58 +++++++++++++++++++++++++++++++++++++++++-----------------
 reglib.h |  5 +++--
 2 files changed, 44 insertions(+), 19 deletions(-)

diff --git a/reglib.c b/reglib.c
index c4d00f8..224821b 100644
--- a/reglib.c
+++ b/reglib.c
@@ -10,6 +10,7 @@
 #include <stdbool.h>
 #include <unistd.h>
 #include <string.h>
+#include <limits.h>
 
 #include <arpa/inet.h> /* ntohl */
 
@@ -37,10 +38,16 @@
 #include "keys-gcrypt.c"
 #endif
 
-void *reglib_get_file_ptr(uint8_t *db, int dblen, int structlen, uint32_t ptr)
+void *
+reglib_get_file_ptr(uint8_t *db, size_t dblen, size_t structlen, uint32_t ptr)
 {
 	uint32_t p = ntohl(ptr);
 
+	if (structlen > dblen) {
+		fprintf(stderr, "Invalid database file, too short!\n");
+		exit(3);
+	}
+
 	if (p > dblen - structlen) {
 		fprintf(stderr, "Invalid database file, bad pointer!\n");
 		exit(3);
@@ -49,6 +56,17 @@ void *reglib_get_file_ptr(uint8_t *db, int dblen, int structlen, uint32_t ptr)
 	return (void *)(db + p);
 }
 
+static size_t
+reglib_array_len(size_t baselen, unsigned int elemcount, size_t elemlen)
+{
+	if (elemcount > (SIZE_MAX - baselen) / elemlen) {
+		fprintf(stderr, "Invalid database file, count too large!\n");
+		exit(3);
+	}
+
+	return baselen + elemcount * elemlen;
+}
+
 /*
  * reglib_verify_db_signature():
  *
@@ -59,7 +77,7 @@ void *reglib_get_file_ptr(uint8_t *db, int dblen, int structlen, uint32_t ptr)
  */
 
 #ifdef USE_OPENSSL
-int reglib_verify_db_signature(uint8_t *db, int dblen, int siglen)
+int reglib_verify_db_signature(uint8_t *db, size_t dblen, size_t siglen)
 {
 	RSA *rsa;
 	uint8_t hash[SHA_DIGEST_LENGTH];
@@ -118,7 +136,7 @@ out:
 #endif /* USE_OPENSSL */
 
 #ifdef USE_GCRYPT
-int reglib_verify_db_signature(uint8_t *db, int dblen, int siglen)
+int reglib_verify_db_signature(uint8_t *db, size_t dblen, size_t siglen)
 {
 	gcry_mpi_t mpi_e, mpi_n;
 	gcry_sexp_t rsa, signature, data;
@@ -180,7 +198,7 @@ out:
 #endif /* USE_GCRYPT */
 
 #if !defined(USE_OPENSSL) && !defined(USE_GCRYPT)
-int reglib_verify_db_signature(uint8_t *db, int dblen, int siglen)
+int reglib_verify_db_signature(uint8_t *db, size_t dblen, size_t siglen)
 {
 	return 1;
 }
@@ -220,7 +238,7 @@ const struct reglib_regdb_ctx *reglib_malloc_regdb_ctx(const char *regdb_file)
 		return NULL;
 	}
 
-	ctx->header = reglib_get_file_ptr(ctx->db, ctx->dblen,
+	ctx->header = reglib_get_file_ptr(ctx->db, ctx->real_dblen,
 					  sizeof(struct regdb_file_header),
 					  0);
 	header = ctx->header;
@@ -232,12 +250,13 @@ const struct reglib_regdb_ctx *reglib_malloc_regdb_ctx(const char *regdb_file)
 		goto err_out;
 
 	ctx->siglen = ntohl(header->signature_length);
-	/* The actual dblen does not take into account the signature */
-	ctx->dblen = ctx->real_dblen - ctx->siglen;
 
-	if (ctx->dblen <= sizeof(*header))
+	if (ctx->siglen > ctx->real_dblen - sizeof(*header))
 		goto err_out;
 
+	/* The actual dblen does not take into account the signature */
+	ctx->dblen = ctx->real_dblen - ctx->siglen;
+
 	/* verify signature */
 	if (!reglib_verify_db_signature(ctx->db, ctx->dblen, ctx->siglen))
 		goto err_out;
@@ -272,7 +291,7 @@ void reglib_free_regdb_ctx(const struct reglib_regdb_ctx *regdb_ctx)
 	free(ctx);
 }
 
-static void reg_rule2rd(uint8_t *db, int dblen,
+static void reg_rule2rd(uint8_t *db, size_t dblen,
 	uint32_t ruleptr, struct ieee80211_reg_rule *rd_reg_rule)
 {
 	struct regdb_file_reg_rule *rule;
@@ -303,18 +322,21 @@ country2rd(const struct reglib_regdb_ctx *ctx,
 {
 	struct regdb_file_reg_rules_collection *rcoll;
 	struct ieee80211_regdomain *rd;
-	int i, num_rules, size_of_rd;
+	unsigned int i, num_rules;
+	size_t size_of_rd;
 
 	rcoll = reglib_get_file_ptr(ctx->db, ctx->dblen, sizeof(*rcoll),
 				    country->reg_collection_ptr);
 	num_rules = ntohl(rcoll->reg_rule_num);
 	/* re-get pointer with sanity checking for num_rules */
 	rcoll = reglib_get_file_ptr(ctx->db, ctx->dblen,
-			sizeof(*rcoll) + num_rules * sizeof(uint32_t),
-			country->reg_collection_ptr);
+				    reglib_array_len(sizeof(*rcoll), num_rules,
+						     sizeof(uint32_t)),
+				    country->reg_collection_ptr);
 
-	size_of_rd = sizeof(struct ieee80211_regdomain) +
-		num_rules * sizeof(struct ieee80211_reg_rule);
+	size_of_rd = reglib_array_len(sizeof(struct ieee80211_regdomain),
+				      num_rules,
+				      sizeof(struct ieee80211_reg_rule));
 
 	rd = malloc(size_of_rd);
 	if (!rd)
@@ -468,7 +490,8 @@ struct ieee80211_regdomain *
 reglib_intersect_rds(const struct ieee80211_regdomain *rd1,
 		     const struct ieee80211_regdomain *rd2)
 {
-	int r, size_of_regd;
+	int r;
+	size_t size_of_regd;
 	unsigned int x, y;
 	unsigned int num_rules = 0, rule_idx = 0;
 	const struct ieee80211_reg_rule *rule1, *rule2;
@@ -506,8 +529,9 @@ reglib_intersect_rds(const struct ieee80211_regdomain *rd1,
 	if (!num_rules)
 		return NULL;
 
-	size_of_regd = sizeof(struct ieee80211_regdomain) +
-		((num_rules + 1) * sizeof(struct ieee80211_reg_rule));
+	size_of_regd = reglib_array_len(sizeof(struct ieee80211_regdomain),
+					num_rules + 1,
+					sizeof(struct ieee80211_reg_rule));
 
 	rd = malloc(size_of_regd);
 	if (!rd)
diff --git a/reglib.h b/reglib.h
index 86087e3..57082fe 100644
--- a/reglib.h
+++ b/reglib.h
@@ -112,8 +112,9 @@ static inline uint32_t reglib_min(uint32_t a, uint32_t b)
 	return (a > b) ? b : a;
 }
 
-void *reglib_get_file_ptr(uint8_t *db, int dblen, int structlen, uint32_t ptr);
-int reglib_verify_db_signature(uint8_t *db, int dblen, int siglen);
+void *
+reglib_get_file_ptr(uint8_t *db, size_t dblen, size_t structlen, uint32_t ptr);
+int reglib_verify_db_signature(uint8_t *db, size_t dblen, size_t siglen);
 
 /**
  * reglib_malloc_regdb_ctx - create a regdb context for usage with reglib

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 828 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/wireless-regdb/attachments/20130701/8de01883/attachment-0001.sig>


More information about the wireless-regdb mailing list