From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Date: Mon, 19 Oct 2020 20:42:56 +0000 (+1300)
Subject: rpc: avoid undefined behaviour when parsing bindings
X-Git-Tag: talloc-2.3.2~179
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=302098c3259c3709f61e5f2859785dbb62a393e5;p=thirdparty%2Fsamba.git

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 aa8cc6b46c6..75246dfd538 100644
--- 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, '@');