From 52602e4f5ad0f7c3cdb4a50dfe32d0b8ad49b6e4 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 28 Mar 2011 13:26:27 -0700 Subject: 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. --- libcli/auth/msrpc_parse.c | 34 ++++++++++++++++++++++++++++------ libcli/auth/msrpc_parse.h | 2 +- libcli/auth/ntlmssp_server.c | 13 +++++++++++-- libcli/auth/ntlmssp_sign.c | 16 ++++++++-------- libcli/auth/smbencrypt.c | 6 ++++-- 5 files changed, 52 insertions(+), 19 deletions(-) (limited to 'libcli/auth') diff --git a/libcli/auth/msrpc_parse.c b/libcli/auth/msrpc_parse.c index 1351dfaae7..bdbba3d76c 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 507694db70..d976a95b73 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 264e8bc908..802ac402b4 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 0e57c07a8d..42b459c6d4 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 abd8ad92a3..825739ac4b 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 */ -- cgit