[PATCH] Fix undefined behavior in NDR

[Andreas Schneider]

On Wednesday, 5 December 2018 22:32:17 CET Stefan Metzmacher via samba-
technical wrote:
> Hi Andreas,

Hi metze,

> Why do we need this if we have checks in ndr_push_bytes()?
> 
> Adding the above two checks in ndr_push_bytes() might also be very
> costly. Isn't the first check enough? It should also use unlikely()

updated patchset attached ...



	Andreas

-- 
Andreas Schneider                      asn at samba.org
Samba Team                             www.samba.org
GPG-ID:     8DFF53E18F2ABC8D8F3C92237EE0FC4DCC014E3D
-------------- next part --------------
>From 519b65aa9e5e69b1ef4d1351f6e62455f0e0f6f2 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 22 Nov 2018 15:15:03 +0100
Subject: [PATCH] librpc:ndr: Fix undefined behavior in ndr_basic

librpc/ndr/ndr_basic.c:723:2: runtime error: null pointer passed as
argument 2, which is declared to never be null

The following triggered the undefined behavior:

(gdb) bt
    at librpc/gen_ndr/ndr_drsuapi.c:2318
    fn=0x7ffff6e72983 <ndr_push_drsuapi_DsReplicaObjectIdentifier3Binary>) at ../../librpc/ndr/ndr.c:1337
    at ../../source4/dsdb/schema/schema_syntax.c:2136
    drs_str=<optimized out>) at ../../source4/dsdb/schema/tests/schema_syntax.c:122
    already_setup=<optimized out>, restricted=restricted at entry=0x0) at ../../lib/torture/torture.c:442
    at ../../lib/torture/torture.c:507
    suite=0x5555563d9490, matched=0x7fffffffcef7) at ../../source4/torture/smbtorture.c:93
    matched=0x7fffffffcef7) at ../../source4/torture/smbtorture.c:95
    at ../../source4/torture/smbtorture.c:143
(gdb) f 1
1335            NDR_CHECK(ndr_push_bytes(ndr, blob.data, blob.length));
(gdb) p blob
$2 = {data = 0x0, length = 0}

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 librpc/ndr/ndr_basic.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/librpc/ndr/ndr_basic.c b/librpc/ndr/ndr_basic.c
index c874f340388..3a5189570c5 100644
--- a/librpc/ndr/ndr_basic.c
+++ b/librpc/ndr/ndr_basic.c
@@ -719,6 +719,12 @@ _PUBLIC_ enum ndr_err_code ndr_pull_trailer_align(struct ndr_pull *ndr, size_t s
 */
 _PUBLIC_ enum ndr_err_code ndr_push_bytes(struct ndr_push *ndr, const uint8_t *data, uint32_t n)
 {
+	if (unlikely(n == 0)) {
+		return NDR_ERR_SUCCESS;
+	}
+	if (unlikely(data == NULL)) {
+		return NDR_ERR_INVALID_POINTER;
+	}
 	NDR_PUSH_NEED_BYTES(ndr, n);
 	memcpy(ndr->data + ndr->offset, data, n);
 	ndr->offset += n;
-- 
2.19.2