libcli/smb: Fix alignment problems of smb_bytes_pull_str()

This function needs to get the whole smb buffer in order to get
the alignment for unicode correct.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12824

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>

3 files changed, 47 insertions, 31 deletions

diff --git a/libcli/smb/smb1cli_session.c b/libcli/smb/smb1cli_session.c
index 9d92aa6aed4..11614df0ae4 100644
--- a/libcli/smb/smb1cli_session.c
+++ b/libcli/smb/smb1cli_session.c
@@ -210,16 +210,16 @@ static void smb1cli_session_setup_lm21_done(struct tevent_req *subreq)
 	p = bytes;
 
 	status = smb_bytes_pull_str(state, &state->out_native_os,
-				    use_unicode, p,
-				    bytes+num_bytes-p, &ret);
+				    use_unicode, bytes, num_bytes,
+				    p, &ret);
 	if (tevent_req_nterror(req, status)) {
 		return;
 	}
 	p += ret;
 
 	status = smb_bytes_pull_str(state, &state->out_native_lm,
-				    use_unicode, p,
-				    bytes+num_bytes-p, &ret);
+				    use_unicode, bytes, num_bytes,
+				    p, &ret);
 	if (tevent_req_nterror(req, status)) {
 		return;
 	}
@@ -493,24 +493,24 @@ static void smb1cli_session_setup_nt1_done(struct tevent_req *subreq)
 	p = bytes;
 
 	status = smb_bytes_pull_str(state, &state->out_native_os,
-				    use_unicode, p,
-				    bytes+num_bytes-p, &ret);
+				    use_unicode, bytes, num_bytes,
+				    p, &ret);
 	if (tevent_req_nterror(req, status)) {
 		return;
 	}
 	p += ret;
 
 	status = smb_bytes_pull_str(state, &state->out_native_lm,
-				    use_unicode, p,
-				    bytes+num_bytes-p, &ret);
+				    use_unicode, bytes, num_bytes,
+				    p, &ret);
 	if (tevent_req_nterror(req, status)) {
 		return;
 	}
 	p += ret;
 
 	status = smb_bytes_pull_str(state, &state->out_primary_domain,
-				    use_unicode, p,
-				    bytes+num_bytes-p, &ret);
+				    use_unicode, bytes, num_bytes,
+				    p, &ret);
 	if (tevent_req_nterror(req, status)) {
 		return;
 	}
@@ -754,16 +754,16 @@ static void smb1cli_session_setup_ext_done(struct tevent_req *subreq)
 	p += out_security_blob_length;
 
 	status = smb_bytes_pull_str(state, &state->out_native_os,
-				    use_unicode, p,
-				    bytes+num_bytes-p, &ret);
+				    use_unicode, bytes, num_bytes,
+				    p, &ret);
 	if (tevent_req_nterror(req, status)) {
 		return;
 	}
 	p += ret;
 
 	status = smb_bytes_pull_str(state, &state->out_native_lm,
-				    use_unicode, p,
-				    bytes+num_bytes-p, &ret);
+				    use_unicode, bytes, num_bytes,
+				    p, &ret);
 	if (tevent_req_nterror(req, status)) {
 		return;
 	}
diff --git a/libcli/smb/smb_util.h b/libcli/smb/smb_util.h
index 7e6f0a4ebc4..2884786339d 100644
--- a/libcli/smb/smb_util.h
+++ b/libcli/smb/smb_util.h
@@ -38,4 +38,5 @@ uint8_t *trans2_bytes_push_bytes(uint8_t *buf,
 				 const uint8_t *bytes, size_t num_bytes);
 NTSTATUS smb_bytes_pull_str(TALLOC_CTX *mem_ctx, char **_str, bool ucs2,
 			    const uint8_t *buf, size_t buf_len,
-			    size_t *pbuf_consumed);
+			    const uint8_t *position,
+			    size_t *_consumed);
diff --git a/libcli/smb/util.c b/libcli/smb/util.c
index ef8c9fafa35..7ef909c6077 100644
--- a/libcli/smb/util.c
+++ b/libcli/smb/util.c
@@ -319,29 +319,43 @@ uint8_t *trans2_bytes_push_bytes(uint8_t *buf,
 static NTSTATUS internal_bytes_pull_str(TALLOC_CTX *mem_ctx, char **_str,
 					bool ucs2, bool align_odd,
 					const uint8_t *buf, size_t buf_len,
-					size_t *pbuf_consumed)
+					const uint8_t *position,
+					size_t *p_consumed)
 {
 	size_t pad = 0;
+	size_t offset;
 	char *str = NULL;
 	size_t str_len = 0;
 	bool ok;
 
 	*_str = NULL;
-	if (pbuf_consumed != NULL) {
-		*pbuf_consumed = 0;
+	if (p_consumed != NULL) {
+		*p_consumed = 0;
+	}
+
+	if (position < buf) {
+		return NT_STATUS_INTERNAL_ERROR;
+	}
+
+	offset = PTR_DIFF(position, buf);
+	if (offset > buf_len) {
+		return NT_STATUS_BUFFER_TOO_SMALL;
 	}
 
 	if (ucs2 &&
-	    ((align_odd && (buf_len % 2 == 0)) ||
-	     (!align_odd && (buf_len % 2 == 1)))) {
-		if (buf_len < 1) {
-			return NT_STATUS_BUFFER_TOO_SMALL;
-		}
-		pad = 1;
-		buf_len -= pad;
-		buf += pad;
+	    ((align_odd && (offset % 2 == 0)) ||
+	     (!align_odd && (offset % 2 == 1)))) {
+		pad += 1;
+		offset += 1;
+	}
+
+	if (offset > buf_len) {
+		return NT_STATUS_BUFFER_TOO_SMALL;
 	}
 
+	buf_len -= offset;
+	buf += offset;
+
 	if (ucs2) {
 		buf_len = utf16_len_n(buf, buf_len);
 	} else {
@@ -361,17 +375,18 @@ static NTSTATUS internal_bytes_pull_str(TALLOC_CTX *mem_ctx, char **_str,
 		return map_nt_error_from_unix_common(errno);
 	}
 
-	if (pbuf_consumed != NULL) {
-		*pbuf_consumed = buf_len + pad;
+	if (p_consumed != NULL) {
+		*p_consumed = buf_len + pad;
 	}
 	*_str = str;
-	return NT_STATUS_OK;;
+	return NT_STATUS_OK;
 }
 
 NTSTATUS smb_bytes_pull_str(TALLOC_CTX *mem_ctx, char **_str, bool ucs2,
 			    const uint8_t *buf, size_t buf_len,
-			    size_t *_buf_consumed)
+			    const uint8_t *position,
+			    size_t *_consumed)
 {
 	return internal_bytes_pull_str(mem_ctx, _str, ucs2, true,
-				       buf, buf_len, _buf_consumed);
+				       buf, buf_len, position, _consumed);
 }