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>

diff --git a/libcli/smb/smb1cli_session.c b/libcli/smb/smb1cli_session.c
index 9d92aa6aed483ae8d35a775d08cb2e2f4eb40c2e..11614df0ae43764d3024290c6567860e878f6aa1 100644 (file)
--- 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 7e6f0a4ebc437cfbfd70198c3038599856e2b21b..2884786339de1f63e96c98c3c68f28c173379bd3 100644 (file)
--- 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 ef8c9fafa3561720bbe29f354a5cc14a677b3023..7ef909c60773a2fc2bfabdd4db61a388d9087403 100644 (file)
--- 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);
 }