From: Jeremy Allison Date: Mon, 28 Mar 2011 20:26:27 +0000 (-0700) Subject: Fix inspired by work done by David Disseldorp for bug #8040 - smbclient segfaults... X-Git-Tag: ldb-1.1.0~489 X-Git-Url: http://git.samba.org/samba.git/?a=commitdiff_plain;ds=sidebyside;h=52602e4f5ad0f7c3cdb4a50dfe32d0b8ad49b6e4;p=ira%2Fwip.git Fix inspired by work done by David Disseldorp for bug #8040 - smbclient segfaults when a Cyrillic netbios name or workgroup is configured. 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 1351dfaae78..bdbba3d76cc 100644 --- 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 507694db706..d976a95b737 100644 --- 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 264e8bc9084..802ac402b49 100644 --- 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 0e57c07a8d5..42b459c6d47 100644 --- 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 abd8ad92a3c..825739ac4be 100644 --- 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 7a006a373f9..e0bcccaee63 100644 --- 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 13827e9c96d..53bd7a4d238 100644 --- 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;