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.

diff --git a/p11-kit/test-uri.c b/p11-kit/test-uri.c
index 1fb50810ab5f4d60b17ec0d884e1e311aefedf89..7c38e6026865e66e9f87e6f5351c19a56640fd62 100644 (file)
--- 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 ddb29a50c22ea01eb7cf3061bf23588d86fc6307..832980d21f89740857c564188aa6511975e1c89f 100644 (file)
--- 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);