The branch, master has been updated
via 91d5ea2 librpc/ndr/uuid.c: improve speed and accuracy of GUID
string parsing
via 6c9a185 s4-torture: better, failing, tests for GUID_from_string
from c0549ae cli-quotas: fix potential memory leak
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 91d5ea2ae90140cad0fa8021f07dad3f3d7b7734
Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Date: Wed Dec 7 11:54:41 2016 +1300
librpc/ndr/uuid.c: improve speed and accuracy of GUID string parsing
GUID_from_data_blob() was relying on sscanf to parse strings, which was
slow and quite accepting of invalid GUIDs. Instead we directly read a
fixed number of hex bytes for each field.
This now passes the samba4.local.ndr.*.guid_from_string_invalid tests.
Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
Autobuild-User(master): Douglas Bagnall <dbagn...@samba.org>
Autobuild-Date(master): Wed Dec 14 08:55:42 CET 2016 on sn-devel-144
commit 6c9a185be260a914bc0bd2dcf76c9dcb9664a687
Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Date: Wed Dec 7 14:35:58 2016 +1300
s4-torture: better, failing, tests for GUID_from_string
These tests reveal that the current implementation accepts all kinds
of invalid GUIDs. In particular, we fail on these ones:
"00000001-0002-0003-0405--060708090a0"
"-0000001-0002-0003-0405-060708090a0b"
"-0000001-0002-0003-04-5-060708090a0b"
"d0000001-0002-0003-0405-060708090a-b"
"00000001- -2-0003-0405-060708090a0b"
"00000001-0002-0003-0405- 060708090a0"
"0x000001-0002-0003-0405-060708090a0b"
"00000001-0x02-0x03-0405-060708090a0b"
This test is added to selftest/knownfail.
The test for valid string GUIDs is extended to test upper and mixed case
GUIDs.
Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
-----------------------------------------------------------------------
Summary of changes:
librpc/ndr/uuid.c | 132 ++++++++++++++++++++++++++++++++++++++--------
source4/torture/ndr/ndr.c | 64 ++++++++++++++++++----
2 files changed, 166 insertions(+), 30 deletions(-)
Changeset truncated at 500 lines:
diff --git a/librpc/ndr/uuid.c b/librpc/ndr/uuid.c
index a3f68d1..037dd90 100644
--- a/librpc/ndr/uuid.c
+++ b/librpc/ndr/uuid.c
@@ -52,6 +52,102 @@ _PUBLIC_ NTSTATUS GUID_from_ndr_blob(const DATA_BLOB *b,
struct GUID *guid)
return ndr_map_error2ntstatus(ndr_err);
}
+static NTSTATUS read_hex_bytes(const char *s, uint hexchars, uint64_t *dest)
+{
+ uint64_t x = 0;
+ uint i;
+ char c;
+
+ if ((hexchars & 1) || hexchars > 16) {
+ return NT_STATUS_INVALID_PARAMETER;
+ }
+
+ for (i = 0; i < hexchars; i++) {
+ x <<= 4;
+ c = s[i];
+ if (c >= '0' && c <= '9') {
+ x += c - '0';
+ }
+ else if (c >= 'a' && c <= 'f') {
+ x += c - 'a' + 10;
+ }
+ else if (c >= 'A' && c <= 'F') {
+ x += c - 'A' + 10;
+ }
+ else {
+ /* BAD character (including '\0') */
+ return NT_STATUS_INVALID_PARAMETER;
+ }
+ }
+ *dest = x;
+ return NT_STATUS_OK;
+}
+
+
+static NTSTATUS parse_guid_string(const char *s,
+ uint32_t *time_low,
+ uint32_t *time_mid,
+ uint32_t *time_hi_and_version,
+ uint32_t *clock_seq,
+ uint32_t *node)
+{
+ uint64_t tmp;
+ NTSTATUS status;
+ int i;
+ /* "e12b56b6-0a95-11d1-adbb-00c04fd8d5cd"
+ | | | | |
+ | | | | \ node[6]
+ | | | \_____ clock_seq[2]
+ | | \__________ time_hi_and_version
+ | \_______________ time_mid
+ \_____________________ time_low
+ */
+ status = read_hex_bytes(s, 8, &tmp);
+ if (!NT_STATUS_IS_OK(status) || s[8] != '-') {
+ return NT_STATUS_INVALID_PARAMETER;
+ }
+ *time_low = tmp;
+ s += 9;
+
+ status = read_hex_bytes(s, 4, &tmp);
+ if (!NT_STATUS_IS_OK(status) || s[4] != '-') {
+ return NT_STATUS_INVALID_PARAMETER;
+ }
+ *time_mid = tmp;
+ s += 5;
+
+ status = read_hex_bytes(s, 4, &tmp);
+ if (!NT_STATUS_IS_OK(status) || s[4] != '-') {
+ return NT_STATUS_INVALID_PARAMETER;
+ }
+ *time_hi_and_version = tmp;
+ s += 5;
+
+ for (i = 0; i < 2; i++) {
+ status = read_hex_bytes(s, 2, &tmp);
+ if (!NT_STATUS_IS_OK(status)) {
+ return NT_STATUS_INVALID_PARAMETER;
+ }
+ clock_seq[i] = tmp;
+ s += 2;
+ }
+ if (s[0] != '-') {
+ return NT_STATUS_INVALID_PARAMETER;
+ }
+
+ s++;
+
+ for (i = 0; i < 6; i++) {
+ status = read_hex_bytes(s, 2, &tmp);
+ if (!NT_STATUS_IS_OK(status)) {
+ return NT_STATUS_INVALID_PARAMETER;
+ }
+ node[i] = tmp;
+ s += 2;
+ }
+
+ return NT_STATUS_OK;
+}
/**
build a GUID from a string
@@ -75,32 +171,26 @@ _PUBLIC_ NTSTATUS GUID_from_data_blob(const DATA_BLOB *s,
struct GUID *guid)
switch(s->length) {
case 36:
{
- char string[37];
- memcpy(string, s->data, 36);
- string[36] = 0;
-
- if (11 == sscanf(string,
-
"%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
- &time_low, &time_mid, &time_hi_and_version,
- &clock_seq[0], &clock_seq[1],
- &node[0], &node[1], &node[2], &node[3],
&node[4], &node[5])) {
- status = NT_STATUS_OK;
- }
+ status = parse_guid_string((char *)s->data,
+ &time_low,
+ &time_mid,
+ &time_hi_and_version,
+ clock_seq,
+ node);
break;
}
case 38:
{
- char string[39];
- memcpy(string, s->data, 38);
- string[38] = 0;
-
- if (11 == sscanf(string,
-
"{%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x}",
- &time_low, &time_mid, &time_hi_and_version,
- &clock_seq[0], &clock_seq[1],
- &node[0], &node[1], &node[2], &node[3],
&node[4], &node[5])) {
- status = NT_STATUS_OK;
+ if (s->data[0] != '{' || s->data[37] != '}') {
+ break;
}
+
+ status = parse_guid_string((char *)s->data + 1,
+ &time_low,
+ &time_mid,
+ &time_hi_and_version,
+ clock_seq,
+ node);
break;
}
case 32:
diff --git a/source4/torture/ndr/ndr.c b/source4/torture/ndr/ndr.c
index 7d28eb0..d67585c 100644
--- a/source4/torture/ndr/ndr.c
+++ b/source4/torture/ndr/ndr.c
@@ -296,17 +296,61 @@ static bool test_guid_from_string_null(struct
torture_context *tctx)
static bool test_guid_from_string_invalid(struct torture_context *tctx)
{
struct GUID g1;
- torture_assert_ntstatus_equal(tctx, NT_STATUS_INVALID_PARAMETER,
- GUID_from_string("bla", &g1),
- "parameter not invalid");
+ bool failed = false;
+ int i;
+ const char *bad_guids[] = {
+ "bla",
+ "",
+ /*
+ "00000001-0002-0003-0405-060708090a0b", correct
+ */
+ "00000001-0002-0003-0405-060708090a0b1", /* too long */
+ "00000001-0002-0003-0405-060708090a0", /* too short */
+ "00000001-0002-0003-0405--060708090a0", /* negative */
+ "00000001-0002-0003--0405-060708090a0", /* negative */
+ "-0000001-0002-0003-0405-060708090a0b", /* negative */
+ "-0000001-0002-0003-04-5-060708090a0b", /* negative */
+ "d0000001-0002-0003-0405-060708090a-b", /* negative */
+ "00000001- -2-0003-0405-060708090a0b", /* negative, space */
+ "00000001-0002-0003-0405- 060708090a0", /* whitespace */
+ " 0000001-0002-0003--0405-060708090a0", /* whitespace */
+ "00000001-0002-0003--0405-060708090a ", /* whitespace */
+ "0000001-00002-0003-04050-60708090a0b", /* misshapen */
+ "00000010-0002-0003-04050-60708090a0b", /* misshapen */
+ "00000001-0002-0003-0405-0z0708090a0b", /* bad char */
+ "00000001-00x2-0x03-0405-060708090a0b", /* bad char (00x) */
+ "0x000001-0002-0003-0405-060708090a0b", /* 0x char */
+ "00000001-0x02-0x03-0405-060708090a0b", /* 0x char */
+ };
+
+ for (i = 0; i < ARRAY_SIZE(bad_guids); i++) {
+ NTSTATUS status = GUID_from_string(bad_guids[i], &g1);
+ if (! NT_STATUS_EQUAL(status, NT_STATUS_INVALID_PARAMETER)) {
+ torture_comment(tctx, "bad guid %s parsed as OK\n",
+ bad_guids[i]);
+ failed = true;
+ }
+ }
+ if (failed) {
+ torture_fail(tctx, "wrongly allowing invalid guids");
+ }
return true;
}
static bool test_guid_from_string(struct torture_context *tctx)
{
struct GUID g1, exp;
+ /* we are asserting all these guids are valid and equal */
+ const char *guids[4] = {
+ "00000001-0002-0003-0405-060708090a0b",
+ "{00000001-0002-0003-0405-060708090a0b}",
+ "{00000001-0002-0003-0405-060708090a0B}", /* mixed */
+ "00000001-0002-0003-0405-060708090A0B", /* upper */
+ };
+ int i;
+
torture_assert_ntstatus_ok(tctx,
-
GUID_from_string("00000001-0002-0003-0405-060708090a0b", &g1),
+ GUID_from_string(guids[0], &g1),
"invalid return code");
exp.time_low = 1;
exp.time_mid = 2;
@@ -319,12 +363,14 @@ static bool test_guid_from_string(struct torture_context
*tctx)
exp.node[3] = 9;
exp.node[4] = 10;
exp.node[5] = 11;
- torture_assert(tctx, GUID_equal(&g1, &exp), "UUID parsed incorrectly");
- torture_assert_ntstatus_ok(tctx,
-
GUID_from_string("{00000001-0002-0003-0405-060708090a0b}", &g1),
- "invalid return code");
- torture_assert(tctx, GUID_equal(&g1, &exp), "UUID parsed incorrectly");
+ for (i = 1; i < ARRAY_SIZE(guids); i++) {
+ torture_assert_ntstatus_ok(tctx,
+ GUID_from_string(guids[i], &g1),
+ "invalid return code");
+ torture_assert(tctx, GUID_equal(&g1, &exp),
+ "UUID parsed incorrectly");
+ }
return true;
}
--
Samba Shared Repository
