rpc: avoid undefined behaviour when parsing bindings

If the binding string ends with "[", we were setting options to an
empty string, then asking for 'options[strlen(options)-1]', which
UBSan dosn't like because the offset evaluates to (size_t)0xFFFFF...
causing pointer overflow.

I believe this is actually well defined in practice, but we don't want
to be in the habit of leaving sanitiser warnings in code parsing
untrusted strings.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

diff --git a/librpc/rpc/binding.c b/librpc/rpc/binding.c
index aa8cc6b46c6f151d5103098130c5d071b16bb183..75246dfd5380d9ee9e041e2ef71545e77f897cff 100644 (file)
--- a/librpc/rpc/binding.c
+++ b/librpc/rpc/binding.c
@@ -385,13 +385,14 @@ _PUBLIC_ NTSTATUS dcerpc_parse_binding(TALLOC_CTX *mem_ctx, const char *_s, stru
 
        p = strchr(s, '[');
        if (p) {
-               *p = '\0';
-               options = p + 1;
-               if (options[strlen(options)-1] != ']') {
+               char *q = p + strlen(p) - 1;
+               if (*q != ']') {
                        talloc_free(b);
                        return NT_STATUS_INVALID_PARAMETER_MIX;
                }
-               options[strlen(options)-1] = 0;
+               *p = '\0';
+               *q = '\0';
+               options = p + 1;
        }
 
        p = strchr(s, '@');