From cbf1e42e39c030edb3e2c72ae9b4d7dd7ccf3eea Mon Sep 17 00:00:00 2001
From: Lubomir Rintel <lkundrak@v3.sk>
Date: Wed, 28 Dec 2016 16:11:21 +0100
Subject: uri: fix the query attribute parsing

The pin-* attributes belong to the query part. We should not parse them
until we see a '?' and they're separated with a '&'.

This might be an important thing -- some of the query attributes may
have security implications reaching outside scope of the token itself, to the
host system itself. E.g. a pin-source may cause the consumer to access a file
or module-path (unimplemented) execute code. The user may want to just chop the
attribute part off if they want the consumer access the token and not take the
security considerations into account.
---
 p11-kit/test-uri.c |  6 +++---
 p11-kit/uri.c      | 50 +++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 42 insertions(+), 14 deletions(-)

(limited to 'p11-kit')

diff --git a/p11-kit/test-uri.c b/p11-kit/test-uri.c
index 1fb5081..7c38e60 100644
--- a/p11-kit/test-uri.c
+++ b/p11-kit/test-uri.c
@@ -1335,7 +1335,7 @@ test_uri_pin_source (void)
 	assert (strstr (string, "pin-source=%7cmy-pin-file") != NULL);
 	free (string);
 
-	ret = p11_kit_uri_parse ("pkcs11:pin-source=blah%2Fblah", P11_KIT_URI_FOR_ANY, uri);
+	ret = p11_kit_uri_parse ("pkcs11:?pin-source=blah%2Fblah", P11_KIT_URI_FOR_ANY, uri);
 	assert_num_eq (P11_KIT_URI_OK, ret);
 
 	pin_source = p11_kit_uri_get_pin_source (uri);
@@ -1371,7 +1371,7 @@ test_uri_pin_value (void)
 	assert (strstr (string, "pkcs11:pin-value=1%2a%26%23%25%26%40%28") != NULL);
 	free (string);
 
-	ret = p11_kit_uri_parse ("pkcs11:pin-value=blah%2Fblah", P11_KIT_URI_FOR_ANY, uri);
+	ret = p11_kit_uri_parse ("pkcs11:?pin-value=blah%2Fblah", P11_KIT_URI_FOR_ANY, uri);
 	assert_num_eq (P11_KIT_URI_OK, ret);
 
 	pin_value = p11_kit_uri_get_pin_value (uri);
@@ -1389,7 +1389,7 @@ test_uri_pin_value_bad (void)
 	uri = p11_kit_uri_new ();
 	assert_ptr_not_null (uri);
 
-	ret = p11_kit_uri_parse ("pkcs11:pin-value=blahblah%2", P11_KIT_URI_FOR_ANY, uri);
+	ret = p11_kit_uri_parse ("pkcs11:?pin-value=blahblah%2", P11_KIT_URI_FOR_ANY, uri);
 	assert_num_eq (P11_KIT_URI_BAD_ENCODING, ret);
 
 	p11_kit_uri_free (uri);
diff --git a/p11-kit/uri.c b/p11-kit/uri.c
index ddb29a5..832980d 100644
--- a/p11-kit/uri.c
+++ b/p11-kit/uri.c
@@ -1393,17 +1393,14 @@ p11_kit_uri_parse (const char *string, P11KitUriType uri_type,
 	uri->pin_value = NULL;
 	uri->slot_id = (CK_SLOT_ID)-1;
 
+	/* Parse the path. */
 	for (;;) {
-		spos = strchr (string, ';');
-		if (spos == NULL) {
-			spos = string + strlen (string);
-			assert (*spos == '\0');
-			if (spos == string)
-				break;
-		}
+		spos = string + strcspn (string, ";?");
+		if (spos == string)
+			break;
 
 		epos = strchr (string, '=');
-		if (epos == NULL || spos == string || epos == string || epos >= spos) {
+		if (epos == NULL || epos == string || epos >= spos) {
 			free (allocated);
 			return P11_KIT_URI_BAD_SYNTAX;
 		}
@@ -1423,8 +1420,6 @@ p11_kit_uri_parse (const char *string, P11KitUriType uri_type,
 			ret = parse_module_info (string, epos, epos + 1, spos, uri);
 		if (ret == 0 && (uri_type & P11_KIT_URI_FOR_MODULE_WITH_VERSION) == P11_KIT_URI_FOR_MODULE_WITH_VERSION)
 			ret = parse_module_version_info (string, epos, epos + 1, spos, uri);
-		if (ret == 0)
-			ret = parse_extra_info (string, epos, epos + 1, spos, uri);
 
 		if (ret < 0) {
 			free (allocated);
@@ -1433,9 +1428,42 @@ p11_kit_uri_parse (const char *string, P11KitUriType uri_type,
 		if (ret == 0)
 			uri->unrecognized = true;
 
+		string = spos;
 		if (*spos == '\0')
 			break;
-		string = spos + 1;
+		if (*spos == '?')
+			break;
+		string++;
+	}
+
+	/* Parse the query. */
+	for (;;) {
+		if (*string == '\0')
+			break;
+		string++;
+		spos = strchr (string, '&');
+		if (spos == NULL) {
+			spos = string + strlen (string);
+			assert (*spos == '\0');
+			if (spos == string)
+				break;
+		}
+
+		epos = strchr (string, '=');
+		if (epos == NULL || spos == string || epos == string || epos >= spos) {
+			free (allocated);
+			return P11_KIT_URI_BAD_SYNTAX;
+		}
+
+		ret = parse_extra_info (string, epos, epos + 1, spos, uri);
+		if (ret < 0) {
+			free (allocated);
+			return ret;
+		}
+		if (ret == 0)
+			uri->unrecognized = true;
+
+		string = spos;
 	}
 
 	free (allocated);
-- 
cgit v1.1