Fix inspired by work done by David Disseldorp for bug #8040 - smbclient segfaults...

Change msrpc_gen to return NTSTATUS and ensure everywhere this is
used it is correctly checked to return that status.

Jeremy.

diff --git a/libcli/auth/msrpc_parse.c b/libcli/auth/msrpc_parse.c
index 1351dfaae78da76ce62560be5346316ae09b4a08..bdbba3d76ccc6f038f1b1987751266c4e07c1919 100644 (file)
--- a/libcli/auth/msrpc_parse.c
+++ b/libcli/auth/msrpc_parse.c
@@ -40,7 +40,7 @@
   d = word (4 bytes)
   C = constant ascii string
  */
-bool msrpc_gen(TALLOC_CTX *mem_ctx, 
+NTSTATUS msrpc_gen(TALLOC_CTX *mem_ctx, 
               DATA_BLOB *blob,
               const char *format, ...)
 {
@@ -57,7 +57,13 @@ bool msrpc_gen(TALLOC_CTX *mem_ctx,
        DATA_BLOB *pointers;
 
        pointers = talloc_array(mem_ctx, DATA_BLOB, strlen(format));
+       if (!pointers) {
+               return NT_STATUS_NO_MEMORY;
+       }
        intargs = talloc_array(pointers, int, strlen(format));
+       if (!intargs) {
+               return NT_STATUS_NO_MEMORY;
+       }
 
        /* first scan the format to work out the header and body size */
        va_start(ap, format);
@@ -72,7 +78,7 @@ bool msrpc_gen(TALLOC_CTX *mem_ctx,
                                s, &n);
                        if (!ret) {
                                va_end(ap);
-                               return false;
+                               return map_nt_error_from_unix(errno);
                        }
                        pointers[i].length = n;
                        pointers[i].length -= 2;
@@ -86,7 +92,7 @@ bool msrpc_gen(TALLOC_CTX *mem_ctx,
                                s, &n);
                        if (!ret) {
                                va_end(ap);
-                               return false;
+                               return map_nt_error_from_unix(errno);
                        }
                        pointers[i].length = n;
                        pointers[i].length -= 1;
@@ -102,7 +108,7 @@ bool msrpc_gen(TALLOC_CTX *mem_ctx,
                                s, &n);
                        if (!ret) {
                                va_end(ap);
-                               return false;
+                               return map_nt_error_from_unix(errno);
                        }
                        pointers[i].length = n;
                        pointers[i].length -= 2;
@@ -132,13 +138,22 @@ bool msrpc_gen(TALLOC_CTX *mem_ctx,
                        pointers[i].length = strlen(s)+1;
                        head_size += pointers[i].length;
                        break;
+               default:
+                       va_end(ap);
+                       return NT_STATUS_INVALID_PARAMETER;
                }
        }
        va_end(ap);
 
+       if (head_size + data_size == 0) {
+               return NT_STATUS_INVALID_PARAMETER;
+       }
+
        /* allocate the space, then scan the format again to fill in the values */
        *blob = data_blob_talloc(mem_ctx, NULL, head_size + data_size);
-
+       if (!blob->data) {
+               return NT_STATUS_NO_MEMORY;
+       }
        head_ofs = 0;
        data_ofs = head_size;
 
@@ -185,13 +200,16 @@ bool msrpc_gen(TALLOC_CTX *mem_ctx,
                        memcpy(blob->data + head_ofs, pointers[i].data, n);
                        head_ofs += n;
                        break;
+               default:
+                       va_end(ap);
+                       return NT_STATUS_INVALID_PARAMETER;
                }
        }
        va_end(ap);
        
        talloc_free(pointers);
 
-       return true;
+       return NT_STATUS_OK;
 }
 
 
@@ -231,6 +249,10 @@ bool msrpc_parse(TALLOC_CTX *mem_ctx,
        char *p = talloc_array(mem_ctx, char, p_len);
        bool ret = true;
 
+       if (!p) {
+               return false;
+       }
+
        va_start(ap, format);
        for (i=0; format[i]; i++) {
                switch (format[i]) {

diff --git a/libcli/auth/msrpc_parse.h b/libcli/auth/msrpc_parse.h
index 507694db7063aecd2991cebea28abbd785ebb0bc..d976a95b737a1dd58500498b48d9a6191f48f06e 100644 (file)
--- a/libcli/auth/msrpc_parse.h
+++ b/libcli/auth/msrpc_parse.h
@@ -11,7 +11,7 @@
 
 /* The following definitions come from /home/jeremy/src/samba/git/master/source3/../source4/../libcli/auth/msrpc_parse.c  */
 
-bool msrpc_gen(TALLOC_CTX *mem_ctx, 
+NTSTATUS msrpc_gen(TALLOC_CTX *mem_ctx, 
               DATA_BLOB *blob,
               const char *format, ...);
 

diff --git a/libcli/auth/ntlmssp_server.c b/libcli/auth/ntlmssp_server.c
index 264e8bc908475bedb263deb519bd5d95bd2bd55b..802ac402b4969fb861117db53c0b182661b47ad0 100644 (file)
--- a/libcli/auth/ntlmssp_server.c
+++ b/libcli/auth/ntlmssp_server.c
@@ -144,12 +144,15 @@ NTSTATUS ntlmssp_server_negotiate(struct ntlmssp_state *ntlmssp_state,
        /* This creates the 'blob' of names that appears at the end of the packet */
        if (chal_flags & NTLMSSP_NEGOTIATE_TARGET_INFO)
        {
-               msrpc_gen(ntlmssp_state, &struct_blob, "aaaaa",
+               status = msrpc_gen(ntlmssp_state, &struct_blob, "aaaaa",
                          MsvAvNbDomainName, target_name,
                          MsvAvNbComputerName, ntlmssp_state->server.netbios_name,
                          MsvAvDnsDomainName, ntlmssp_state->server.dns_domain,
                          MsvAvDnsComputerName, ntlmssp_state->server.dns_name,
                          MsvAvEOL, "");
+               if (!NT_STATUS_IS_OK(status)) {
+                       return status;
+               }
        } else {
                struct_blob = data_blob_null;
        }
@@ -187,7 +190,7 @@ NTSTATUS ntlmssp_server_negotiate(struct ntlmssp_state *ntlmssp_state,
                        gen_string = "CdAdbddBb";
                }
 
-               msrpc_gen(out_mem_ctx, reply, gen_string,
+               status = msrpc_gen(out_mem_ctx, reply, gen_string,
                        "NTLMSSP",
                        NTLMSSP_CHALLENGE,
                        target_name,
@@ -197,6 +200,12 @@ NTSTATUS ntlmssp_server_negotiate(struct ntlmssp_state *ntlmssp_state,
                        struct_blob.data, struct_blob.length,
                        version_blob.data, version_blob.length);
 
+               if (!NT_STATUS_IS_OK(status)) {
+                       data_blob_free(&version_blob);
+                       data_blob_free(&struct_blob);
+                       return status;
+               }
+
                data_blob_free(&version_blob);
 
                if (DEBUGLEVEL >= 10) {

diff --git a/libcli/auth/ntlmssp_sign.c b/libcli/auth/ntlmssp_sign.c
index 0e57c07a8d550be771b93cf08a8e998778cbcd07..42b459c6d47a0f0ee5b5d71b1d65e4e537f65794 100644 (file)
--- a/libcli/auth/ntlmssp_sign.c
+++ b/libcli/auth/ntlmssp_sign.c
@@ -130,17 +130,17 @@ static NTSTATUS ntlmssp_make_packet_signature(struct ntlmssp_state *ntlmssp_stat
                dump_data_pw("ntlmssp v2 sig ", sig->data, sig->length);
 
        } else {
-               bool ok;
+               NTSTATUS status;
                uint32_t crc;
 
                crc = crc32_calc_buffer(data, length);
 
-               ok = msrpc_gen(sig_mem_ctx,
+               status = msrpc_gen(sig_mem_ctx,
                               sig, "dddd",
                               NTLMSSP_SIGN_VERSION, 0, crc,
                               ntlmssp_state->crypt->ntlm.seq_num);
-               if (!ok) {
-                       return NT_STATUS_NO_MEMORY;
+               if (!NT_STATUS_IS_OK(status)) {
+                       return status;
                }
 
                ntlmssp_state->crypt->ntlm.seq_num++;
@@ -307,17 +307,17 @@ NTSTATUS ntlmssp_seal_packet(struct ntlmssp_state *ntlmssp_state,
                                           sig->data+4, 8);
                }
        } else {
-               bool ok;
+               NTSTATUS status;
                uint32_t crc;
 
                crc = crc32_calc_buffer(data, length);
 
-               ok = msrpc_gen(sig_mem_ctx,
+               status = msrpc_gen(sig_mem_ctx,
                               sig, "dddd",
                               NTLMSSP_SIGN_VERSION, 0, crc,
                               ntlmssp_state->crypt->ntlm.seq_num);
-               if (!ok) {
-                       return NT_STATUS_NO_MEMORY;
+               if (!NT_STATUS_IS_OK(status)) {
+                       return status;
                }
 
                /*

diff --git a/libcli/auth/smbencrypt.c b/libcli/auth/smbencrypt.c
index abd8ad92a3c39d6a2f17805525aea90b80fb0d9f..825739ac4be29e4046d7113611f630f8dd937369 100644 (file)
--- a/libcli/auth/smbencrypt.c
+++ b/libcli/auth/smbencrypt.c
@@ -363,7 +363,8 @@ DATA_BLOB NTLMv2_generate_names_blob(TALLOC_CTX *mem_ctx,
 {
        DATA_BLOB names_blob = data_blob_talloc(mem_ctx, NULL, 0);
 
-       msrpc_gen(mem_ctx, &names_blob,
+       /* Deliberately ignore return here.. */
+       (void)msrpc_gen(mem_ctx, &names_blob,
                  "aaa",
                  MsvAvNbDomainName, domain,
                  MsvAvNbComputerName, hostname,
@@ -386,7 +387,8 @@ static DATA_BLOB NTLMv2_generate_client_data(TALLOC_CTX *mem_ctx, const DATA_BLO
 
        /* See http://www.ubiqx.org/cifs/SMB.html#SMB.8.5 */
 
-       msrpc_gen(mem_ctx, &response, "ddbbdb",
+       /* Deliberately ignore return here.. */
+       (void)msrpc_gen(mem_ctx, &response, "ddbbdb",
                  0x00000101,     /* Header  */
                  0,              /* 'Reserved'  */
                  long_date, 8,   /* Timestamp */

diff --git a/source3/libsmb/ntlmssp.c b/source3/libsmb/ntlmssp.c
index 7a006a373f9f5384722067dbabb26871bcaa2432..e0bcccaee63ec2eb70003f64cf8a1f1f2f44e347 100644 (file)
--- a/source3/libsmb/ntlmssp.c
+++ b/source3/libsmb/ntlmssp.c
@@ -377,6 +377,8 @@ static NTSTATUS ntlmssp_client_initial(struct ntlmssp_state *ntlmssp_state,
                                  TALLOC_CTX *out_mem_ctx, /* Unused at this time */
                                  DATA_BLOB reply, DATA_BLOB *next_request)
 {
+       NTSTATUS status;
+
        if (ntlmssp_state->unicode) {
                ntlmssp_state->neg_flags |= NTLMSSP_NEGOTIATE_UNICODE;
        } else {
@@ -388,12 +390,17 @@ static NTSTATUS ntlmssp_client_initial(struct ntlmssp_state *ntlmssp_state,
        }
 
        /* generate the ntlmssp negotiate packet */
-       msrpc_gen(ntlmssp_state, next_request, "CddAA",
+       status = msrpc_gen(ntlmssp_state, next_request, "CddAA",
                  "NTLMSSP",
                  NTLMSSP_NEGOTIATE,
                  ntlmssp_state->neg_flags,
                  ntlmssp_state->client.netbios_domain,
                  ntlmssp_state->client.netbios_name);
+       if (!NT_STATUS_IS_OK(status)) {
+               DEBUG(0, ("ntlmssp_client_initial: failed to generate "
+                       "ntlmssp negotiate packet\n"));
+               return status;
+       }
 
        if (DEBUGLEVEL >= 10) {
                struct NEGOTIATE_MESSAGE *negotiate = talloc(
@@ -683,7 +690,7 @@ noccache:
        }
 
        /* this generates the actual auth packet */
-       if (!msrpc_gen(ntlmssp_state, next_request, auth_gen_string,
+       nt_status = msrpc_gen(ntlmssp_state, next_request, auth_gen_string,
                       "NTLMSSP",
                       NTLMSSP_AUTH,
                       lm_response.data, lm_response.length,
@@ -692,8 +699,9 @@ noccache:
                       ntlmssp_state->user,
                       ntlmssp_state->client.netbios_name,
                       encrypted_session_key.data, encrypted_session_key.length,
-                      ntlmssp_state->neg_flags)) {
+                      ntlmssp_state->neg_flags);
 
+       if (!NT_STATUS_IS_OK(nt_status)) {
                return NT_STATUS_NO_MEMORY;
        }
 

diff --git a/source4/auth/ntlmssp/ntlmssp_client.c b/source4/auth/ntlmssp/ntlmssp_client.c
index 13827e9c96d40fa8dde674cbe34a9bdf7c38718a..53bd7a4d238d8e50967a770d6f8d73ca1641c97f 100644 (file)
--- a/source4/auth/ntlmssp/ntlmssp_client.c
+++ b/source4/auth/ntlmssp/ntlmssp_client.c
@@ -54,6 +54,7 @@ NTSTATUS ntlmssp_client_initial(struct gensec_security *gensec_security,
        struct ntlmssp_state *ntlmssp_state = gensec_ntlmssp->ntlmssp_state;
        const char *domain = ntlmssp_state->domain;
        const char *workstation = cli_credentials_get_workstation(gensec_security->credentials);
+       NTSTATUS status;
 
        /* These don't really matter in the initial packet, so don't panic if they are not set */
        if (!domain) {
@@ -75,7 +76,7 @@ NTSTATUS ntlmssp_client_initial(struct gensec_security *gensec_security,
        }
 
        /* generate the ntlmssp negotiate packet */
-       msrpc_gen(out_mem_ctx, 
+       status = msrpc_gen(out_mem_ctx, 
                  out, "CddAA",
                  "NTLMSSP",
                  NTLMSSP_NEGOTIATE,
@@ -83,6 +84,10 @@ NTSTATUS ntlmssp_client_initial(struct gensec_security *gensec_security,
                  domain, 
                  workstation);
 
+       if (!NT_STATUS_IS_OK(status)) {
+               return status;
+       }
+
        ntlmssp_state->expected_state = NTLMSSP_CHALLENGE;
 
        return NT_STATUS_MORE_PROCESSING_REQUIRED;
@@ -269,7 +274,7 @@ NTSTATUS ntlmssp_client_challenge(struct gensec_security *gensec_security,
        debug_ntlmssp_flags(ntlmssp_state->neg_flags);
 
        /* this generates the actual auth packet */
-       if (!msrpc_gen(mem_ctx, 
+       nt_status = msrpc_gen(mem_ctx, 
                       out, auth_gen_string, 
                       "NTLMSSP", 
                       NTLMSSP_AUTH, 
@@ -279,9 +284,10 @@ NTSTATUS ntlmssp_client_challenge(struct gensec_security *gensec_security,
                       user, 
                       cli_credentials_get_workstation(gensec_security->credentials),
                       encrypted_session_key.data, encrypted_session_key.length,
-                      ntlmssp_state->neg_flags)) {
+                      ntlmssp_state->neg_flags);
+       if (!NT_STATUS_IS_OK(nt_status)) {
                talloc_free(mem_ctx);
-               return NT_STATUS_NO_MEMORY;
+               return nt_status;
        }
 
        ntlmssp_state->session_key = session_key;