diff options
| author | Douglas Bagnall <douglas.bagnall@catalyst.net.nz> | 2020-10-20 09:42:56 +1300 |
|---|---|---|
| committer | Andrew Bartlett <abartlet@samba.org> | 2020-10-23 03:25:35 +0000 |
| commit | 302098c3259c3709f61e5f2859785dbb62a393e5 (patch) | |
| tree | 0b011c00f6850fb001f823845d096d5f9c25ee1a /librpc | |
| parent | 09479bf0ee12b8187736b0d6f4dcf0303569169a (diff) | |
| download | samba-302098c3259c3709f61e5f2859785dbb62a393e5.tar.gz | |
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>
Diffstat (limited to 'librpc')
| -rw-r--r-- | librpc/rpc/binding.c | 9 |
1 files changed, 5 insertions, 4 deletions
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, '@'); |