PATCH: Coverity fixes
Volker Lendecke
Volker.Lendecke at SerNet.DE
Wed Nov 13 00:38:37 MST 2013
Hi!
Attached find a few more coverity fixes.
Please review & push!
Thanks,
Volker
--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
From 0a0ffa87c898b54f6ffaadd5b86923a92a35c6e1 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 11 Nov 2013 20:37:48 +0000
Subject: [PATCH 1/5] libsmb: Fix CID 242665 Out-of-bounds access
Coverity is confused by the dual-use of "pss" as an array of size 1. This
is not strictly a bug here, but it is admittedly a small subtlety. It
should fix a whole bunch of Coverity issues. Normally I would resist to
change our code in response to a deficient static checker, but here I
would vote for this compromise.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/libsmb/cliconnect.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/source3/libsmb/cliconnect.c b/source3/libsmb/cliconnect.c
index 81bc028..3c9d03a 100644
--- a/source3/libsmb/cliconnect.c
+++ b/source3/libsmb/cliconnect.c
@@ -2870,6 +2870,7 @@ static struct tevent_req *cli_connect_sock_send(
struct tevent_req *req, *subreq;
struct cli_connect_sock_state *state;
const char *prog;
+ struct sockaddr_storage *addrs;
unsigned i, num_addrs;
NTSTATUS status;
@@ -2893,7 +2894,6 @@ static struct tevent_req *cli_connect_sock_send(
}
if ((pss == NULL) || is_zero_addr(pss)) {
- struct sockaddr_storage *addrs;
/*
* Here we cheat. resolve_name_list is not async at all. So
@@ -2907,8 +2907,12 @@ static struct tevent_req *cli_connect_sock_send(
tevent_req_nterror(req, status);
return tevent_req_post(req, ev);
}
- pss = addrs;
} else {
+ addrs = talloc_array(state, struct sockaddr_storage, 1);
+ if (tevent_req_nomem(addrs, req)) {
+ return tevent_req_post(req, ev);
+ }
+ addrs[0] = *pss;
num_addrs = 1;
}
@@ -2931,7 +2935,7 @@ static struct tevent_req *cli_connect_sock_send(
}
subreq = smbsock_any_connect_send(
- state, ev, pss, state->called_names, state->called_types,
+ state, ev, addrs, state->called_names, state->called_types,
state->calling_names, NULL, num_addrs, port);
if (tevent_req_nomem(subreq, req)) {
return tevent_req_post(req, ev);
--
1.7.9.5
From cd12ab6a87568737c0097ea8d50fcf345a6ed528 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 11 Nov 2013 21:07:09 +0000
Subject: [PATCH 2/5] heimdal: Fix CID 241943 Uninitialized pointer read
In the error case without EXTRA_ADDRESSES we access ignore_addresses
without initialization
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source4/heimdal/lib/krb5/get_addrs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/source4/heimdal/lib/krb5/get_addrs.c b/source4/heimdal/lib/krb5/get_addrs.c
index 0e2bfcf..765badb 100644
--- a/source4/heimdal/lib/krb5/get_addrs.c
+++ b/source4/heimdal/lib/krb5/get_addrs.c
@@ -130,7 +130,8 @@ find_all_addresses (krb5_context context, krb5_addresses *res, int flags)
/* Allocate storage for them. */
res->val = calloc(num, sizeof(*res->val));
if (res->val == NULL) {
- krb5_free_addresses(context, &ignore_addresses);
+ if (flags & EXTRA_ADDRESSES)
+ krb5_free_addresses(context, &ignore_addresses);
freeifaddrs(ifa0);
krb5_set_error_message(context, ENOMEM, N_("malloc: out of memory", ""));
return ENOMEM;
--
1.7.9.5
From 6e5dfb186bcf13a68a31bf710bdc96e5a38f0e0a Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 11 Nov 2013 21:26:34 +0000
Subject: [PATCH 3/5] tls: Fix some noblank line endings
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source4/lib/tls/tls.c | 65 ++++++++++++++++++++++++-------------------------
1 file changed, 32 insertions(+), 33 deletions(-)
diff --git a/source4/lib/tls/tls.c b/source4/lib/tls/tls.c
index 9a3e610..66c8858 100644
--- a/source4/lib/tls/tls.c
+++ b/source4/lib/tls/tls.c
@@ -1,4 +1,4 @@
-/*
+/*
Unix SMB/CIFS implementation.
transport layer security handling code
@@ -6,17 +6,17 @@
Copyright (C) Andrew Tridgell 2004-2005
Copyright (C) Stefan Metzmacher 2004
Copyright (C) Andrew Bartlett 2006
-
+
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 3 of the License, or
(at your option) any later version.
-
+
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
-
+
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
@@ -115,7 +115,7 @@ static ssize_t tls_pull(gnutls_transport_ptr ptr, void *buf, size_t size)
struct tls_context *tls = talloc_get_type(ptr, struct tls_context);
NTSTATUS status;
size_t nread;
-
+
if (tls->have_first_byte) {
*(uint8_t *)buf = tls->first_byte;
tls->have_first_byte = false;
@@ -166,7 +166,7 @@ static ssize_t tls_push(gnutls_transport_ptr ptr, const void *buf, size_t size)
/* Cope with socket_wrapper 1500 byte chunking for PCAP */
do {
status = socket_send(tls->socket, &b, &nwritten);
-
+
if (NT_STATUS_EQUAL(status, STATUS_MORE_ENTRIES)) {
errno = EAGAIN;
return -1;
@@ -215,7 +215,7 @@ static NTSTATUS tls_handshake(struct tls_context *tls)
if (tls->done_handshake) {
return NT_STATUS_OK;
}
-
+
ret = gnutls_handshake(tls->session);
if (ret == GNUTLS_E_INTERRUPTED || ret == GNUTLS_E_AGAIN) {
if (gnutls_record_get_direction(tls->session) == 1) {
@@ -277,7 +277,7 @@ static NTSTATUS tls_socket_pending(struct socket_context *sock, size_t *npending
/*
receive data either by tls or normal socket_recv
*/
-static NTSTATUS tls_socket_recv(struct socket_context *sock, void *buf,
+static NTSTATUS tls_socket_recv(struct socket_context *sock, void *buf,
size_t wantlen, size_t *nread)
{
int ret;
@@ -328,7 +328,7 @@ static NTSTATUS tls_socket_recv(struct socket_context *sock, void *buf,
/*
send data either by tls or normal socket_recv
*/
-static NTSTATUS tls_socket_send(struct socket_context *sock,
+static NTSTATUS tls_socket_send(struct socket_context *sock,
const DATA_BLOB *blob, size_t *sendlen)
{
NTSTATUS status;
@@ -423,8 +423,8 @@ struct tls_params *tls_initialise(TALLOC_CTX *mem_ctx, struct loadparm_context *
if (ret < 0) goto init_failed;
if (cafile && *cafile) {
- ret = gnutls_certificate_set_x509_trust_file(params->x509_cred, cafile,
- GNUTLS_X509_FMT_PEM);
+ ret = gnutls_certificate_set_x509_trust_file(params->x509_cred, cafile,
+ GNUTLS_X509_FMT_PEM);
if (ret < 0) {
DEBUG(0,("TLS failed to initialise cafile %s\n", cafile));
goto init_failed;
@@ -432,25 +432,24 @@ struct tls_params *tls_initialise(TALLOC_CTX *mem_ctx, struct loadparm_context *
}
if (crlfile && *crlfile) {
- ret = gnutls_certificate_set_x509_crl_file(params->x509_cred,
- crlfile,
+ ret = gnutls_certificate_set_x509_crl_file(params->x509_cred,
+ crlfile,
GNUTLS_X509_FMT_PEM);
if (ret < 0) {
DEBUG(0,("TLS failed to initialise crlfile %s\n", crlfile));
goto init_failed;
}
}
-
- ret = gnutls_certificate_set_x509_key_file(params->x509_cred,
+
+ ret = gnutls_certificate_set_x509_key_file(params->x509_cred,
certfile, keyfile,
GNUTLS_X509_FMT_PEM);
if (ret < 0) {
- DEBUG(0,("TLS failed to initialise certfile %s and keyfile %s\n",
+ DEBUG(0,("TLS failed to initialise certfile %s and keyfile %s\n",
certfile, keyfile));
goto init_failed;
}
-
-
+
ret = gnutls_dh_params_init(¶ms->dh_params);
if (ret < 0) goto init_failed;
@@ -464,14 +463,14 @@ struct tls_params *tls_initialise(TALLOC_CTX *mem_ctx, struct loadparm_context *
goto init_failed;
}
dhparms.size = size;
-
+
ret = gnutls_dh_params_import_pkcs3(params->dh_params, &dhparms, GNUTLS_X509_FMT_PEM);
if (ret < 0) goto init_failed;
} else {
ret = gnutls_dh_params_generate2(params->dh_params, DH_BITS);
if (ret < 0) goto init_failed;
}
-
+
gnutls_certificate_set_dh_params(params->x509_cred, params->dh_params);
params->tls_enabled = true;
@@ -490,18 +489,18 @@ init_failed:
/*
setup for a new connection
*/
-struct socket_context *tls_init_server(struct tls_params *params,
+struct socket_context *tls_init_server(struct tls_params *params,
struct socket_context *socket_ctx,
- struct tevent_fd *fde,
+ struct tevent_fd *fde,
const char *plain_chars)
{
struct tls_context *tls;
int ret;
struct socket_context *new_sock;
NTSTATUS nt_status;
-
- nt_status = socket_create_with_ops(socket_ctx, &tls_socket_ops, &new_sock,
- SOCKET_TYPE_STREAM,
+
+ nt_status = socket_create_with_ops(socket_ctx, &tls_socket_ops, &new_sock,
+ SOCKET_TYPE_STREAM,
socket_ctx->flags | SOCKET_FLAG_ENCRYPT);
if (!NT_STATUS_IS_OK(nt_status)) {
return NULL;
@@ -528,7 +527,7 @@ struct socket_context *tls_init_server(struct tls_params *params,
talloc_set_destructor(tls, tls_destructor);
TLSCHECK(gnutls_set_default_priority(tls->session));
- TLSCHECK(gnutls_credentials_set(tls->session, GNUTLS_CRD_CERTIFICATE,
+ TLSCHECK(gnutls_credentials_set(tls->session, GNUTLS_CRD_CERTIFICATE,
params->x509_cred));
gnutls_certificate_server_set_request(tls->session, GNUTLS_CERT_REQUEST);
gnutls_dh_set_prime_bits(tls->session, DH_BITS);
@@ -551,7 +550,7 @@ struct socket_context *tls_init_server(struct tls_params *params,
tls->have_first_byte = false;
tls->tls_enabled = true;
tls->interrupted = false;
-
+
new_sock->state = SOCKET_STATE_SERVER_CONNECTED;
return new_sock;
@@ -575,9 +574,9 @@ struct socket_context *tls_init_client(struct socket_context *socket_ctx,
const int cert_type_priority[] = { GNUTLS_CRT_X509, GNUTLS_CRT_OPENPGP, 0 };
struct socket_context *new_sock;
NTSTATUS nt_status;
-
- nt_status = socket_create_with_ops(socket_ctx, &tls_socket_ops, &new_sock,
- SOCKET_TYPE_STREAM,
+
+ nt_status = socket_create_with_ops(socket_ctx, &tls_socket_ops, &new_sock,
+ SOCKET_TYPE_STREAM,
socket_ctx->flags | SOCKET_FLAG_ENCRYPT);
if (!NT_STATUS_IS_OK(nt_status)) {
return NULL;
@@ -616,7 +615,7 @@ struct socket_context *tls_init_client(struct socket_context *socket_ctx,
tls->have_first_byte = false;
tls->tls_enabled = true;
tls->interrupted = false;
-
+
new_sock->state = SOCKET_STATE_CLIENT_CONNECTED;
return new_sock;
@@ -685,9 +684,9 @@ struct tls_params *tls_initialise(TALLOC_CTX *mem_ctx, struct loadparm_context *
/*
setup for a new connection
*/
-struct socket_context *tls_init_server(struct tls_params *params,
+struct socket_context *tls_init_server(struct tls_params *params,
struct socket_context *socket,
- struct tevent_fd *fde,
+ struct tevent_fd *fde,
const char *plain_chars)
{
return NULL;
--
1.7.9.5
From b1331d16adbd89998247ef38b752010837707c07 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 11 Nov 2013 21:32:50 +0000
Subject: [PATCH 4/5] tls: Fix CID 242014 Uninitialized scalar variable
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source4/lib/tls/tls.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/source4/lib/tls/tls.c b/source4/lib/tls/tls.c
index 66c8858..b9182ad 100644
--- a/source4/lib/tls/tls.c
+++ b/source4/lib/tls/tls.c
@@ -395,6 +395,7 @@ struct tls_params *tls_initialise(TALLOC_CTX *mem_ctx, struct loadparm_context *
lpcfg_netbios_name(lp_ctx),
lpcfg_dnsdomain(lp_ctx));
if (hostname == NULL) {
+ ret = GNUTLS_E_MEMORY_ERROR;
goto init_failed;
}
tls_cert_generate(params, hostname, keyfile, certfile, cafile);
--
1.7.9.5
From 55f5610c160060783d93a0dc1bf2427a13c79207 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 12 Nov 2013 22:00:54 +0100
Subject: [PATCH 5/5] heimdal: Fix CID 240779 Allocation size mismatch
The error Coverity complains about is in the malloc. krb5_enctypes is
an enum, so it is usually smaller than the size of a pointer. So we
overallocate, but in the memcpy further down we copy from potentially
invalid memory.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source4/heimdal/lib/krb5/context.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/source4/heimdal/lib/krb5/context.c b/source4/heimdal/lib/krb5/context.c
index 99bf1b4..4290b71 100644
--- a/source4/heimdal/lib/krb5/context.c
+++ b/source4/heimdal/lib/krb5/context.c
@@ -450,13 +450,13 @@ copy_etypes (krb5_context context,
;
i++;
- *ret_enctypes = malloc(sizeof(ret_enctypes[0]) * i);
+ *ret_enctypes = malloc(sizeof(enctypes[0]) * i);
if (*ret_enctypes == NULL) {
krb5_set_error_message(context, ENOMEM,
N_("malloc: out of memory", ""));
return ENOMEM;
}
- memcpy(*ret_enctypes, enctypes, sizeof(ret_enctypes[0]) * i);
+ memcpy(*ret_enctypes, enctypes, sizeof(enctypes[0]) * i);
return 0;
}
--
1.7.9.5
More information about the samba-technical
mailing list