[PATCH] Fixes for undefined behavior in util_tdb

[Gary Lockyer]

I'm not sure about the 2nd patch, comments in line.

From 7c7cadda8ab039df4c5eedcf01dea445f737e16b Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 22 Nov 2018 13:33:11 +0100
Subject: [PATCH 2/3] s3:lib: Fix undefined behavior in tdb_pack()

util_tdb.c:98:5: runtime error: null pointer passed as argument 2, which
is declared to never be null

This means the second argument of memcpy() can't be NULL.

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 source3/lib/util_tdb.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/source3/lib/util_tdb.c b/source3/lib/util_tdb.c
index 4f2450c5773..182b27e6376 100644
--- a/source3/lib/util_tdb.c
+++ b/source3/lib/util_tdb.c
@@ -76,14 +76,12 @@ static size_t tdb_pack_va(uint8_t *buf, int bufsize,
const char *fmt, va_list ap
 				SIVAL(buf, 0, d);
 			break;
 		case 'P': /* null-terminated string */
-			s = va_arg(ap,char *);
-			w = strlen(s);
-			len = w + 1;
-			if (bufsize && bufsize >= len)
-				memcpy(buf, s, len);
-			break;
 		case 'f': /* null-terminated string */
 			s = va_arg(ap,char *);
+			if (s == NULL) {
+
				len = 1;
I think this should be an smb_panic, otherwise a random byte will be
silently written to the buffer. While it could write a zero byte, then
we'd be converting a NULL pointer to a zero length string.

+				break;
+			}
 			w = strlen(s);
 			len = w + 1;
 			if (bufsize && bufsize >= len)
@@ -95,7 +93,9 @@ static size_t tdb_pack_va(uint8_t *buf, int bufsize,
const char *fmt, va_list ap
 			len = 4+i;
 			if (bufsize && bufsize >= len) {
 				SIVAL(buf, 0, i);
-				memcpy(buf+4, s, i);
+				if (s != NULL) {
+					memcpy(buf+4, s, i);
+				}
 			}
 			break;
 		default:
-- 
2.19.1

On 28/11/18 04:31, Andreas Schneider via samba-technical wrote:
> Hi,
> 
> see attached patchset. A CI pipeline is here:
> 
> https://gitlab.com/samba-team/devel/samba/pipelines/38025491
> 
> 
> Please review and comment. Push if OK.
> 
> 
> Thanks,
> 
> 
> 	Andreas
> 


Ngā mihi
Gary

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20181130/bded33db/signature.sig>