[PATCH] Fix a few CIDs

Jeremy Allison jra at samba.org
Wed Aug 8 18:19:10 UTC 2018


On Wed, Aug 08, 2018 at 05:52:31PM +0200, Volker Lendecke via samba-technical wrote:
> Hi!
> 
> Plus a small cleanup
> 
> Review appreciated!

LGTM, thanks for catching these. Sorry I missed them
in the original review (but Coverity helps a lot :-).

Noel we probably need these merged into the bug back-port.

Pushed.

Jeremy.

> -- 
> 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
> 
> Meet us at Storage Developer Conference (SDC)
> Santa Clara, CA USA, September 24th-27th 2018

> From 5759e741fa03eec1dc11e18b479d5e3ed4ec2182 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Tue, 7 Aug 2018 22:48:58 +0200
> Subject: [PATCH 1/5] smbd: Align integer types
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
>  source3/smbd/nttrans.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c
> index 5937380fb85..810f8b92b43 100644
> --- a/source3/smbd/nttrans.c
> +++ b/source3/smbd/nttrans.c
> @@ -2301,7 +2301,7 @@ static enum ndr_err_code fill_qtlist_from_sids(TALLOC_CTX *mem_ctx,
>  					       struct dom_sid *sids,
>  					       uint32_t elems)
>  {
> -	int i;
> +	uint32_t i;
>  	TALLOC_CTX *list_ctx = NULL;
>  
>  	list_ctx = talloc_init("quota_sid_list");
> -- 
> 2.11.0
> 
> 
> From a8f98f83a282e66becc843772c0b78e17916c876 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Tue, 7 Aug 2018 22:49:16 +0200
> Subject: [PATCH 2/5] smbd: Fix CID 1438246 Unchecked return value
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
>  source3/smbd/nttrans.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c
> index 810f8b92b43..bad904b9eb8 100644
> --- a/source3/smbd/nttrans.c
> +++ b/source3/smbd/nttrans.c
> @@ -2317,6 +2317,7 @@ static enum ndr_err_code fill_qtlist_from_sids(TALLOC_CTX *mem_ctx,
>  	for (i = 0; i < elems; i++) {
>  		SMB_NTQUOTA_STRUCT qt;
>  		SMB_NTQUOTA_LIST *list_item;
> +		bool ok;
>  
>  		if (!NT_STATUS_IS_OK(vfs_get_ntquota(fsp,
>  						     SMB_USER_QUOTA_TYPE,
> @@ -2333,7 +2334,15 @@ static enum ndr_err_code fill_qtlist_from_sids(TALLOC_CTX *mem_ctx,
>  			return NDR_ERR_ALLOC;
>  		}
>  
> -		sid_to_uid(&sids[i], &list_item->uid);
> +		ok = sid_to_uid(&sids[i], &list_item->uid);
> +		if (!ok) {
> +			char buf[DOM_SID_STR_BUFLEN];
> +			dom_sid_string_buf(&sids[i], buf, sizeof(buf));
> +			DBG_WARNING("Could not convert SID %s to uid\n", buf);
> +			/* No idea what to return here... */
> +			return NDR_ERR_INVALID_POINTER;
> +		}
> +
>  		list_item->quotas = talloc_zero(list_item, SMB_NTQUOTA_STRUCT);
>  		if (list_item->quotas == NULL) {
>  			DBG_ERR("failed to allocate\n");
> -- 
> 2.11.0
> 
> 
> From 2196036e4332c03f057967b5f2fbf18dbf462eaa Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Tue, 7 Aug 2018 22:50:52 +0200
> Subject: [PATCH 3/5] smbd: Fix CID 1438245 Dereference before null check
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
>  source3/smbd/nttrans.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c
> index bad904b9eb8..68470766f77 100644
> --- a/source3/smbd/nttrans.c
> +++ b/source3/smbd/nttrans.c
> @@ -2459,7 +2459,7 @@ static enum ndr_err_code extract_sids_from_buf(TALLOC_CTX *mem_ctx,
>  			}
>  		}
>  		*sids = talloc_zero_array(mem_ctx, struct dom_sid, i);
> -		if (!sids) {
> +		if (*sids == NULL) {
>  			DBG_ERR("OOM\n");
>  			err = NDR_ERR_ALLOC;
>  			goto done;
> -- 
> 2.11.0
> 
> 
> From 7308205b927b11217a166cdbfe37235a13e20fd1 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Wed, 8 Aug 2018 10:08:38 +0200
> Subject: [PATCH 4/5] libsmb: Fix CID 1438244 Unsigned compared against 0
> 
> ndr_size_dom_sid returns a size_t, so that can't be <0. Also, the only
> case that ndr_size_dom_sid returns 0 is a NULL sid
> pointer. ndr_size_dom_sid can reasonably be assumed to not overflow, the
> number of sub-auths is a uint8. That times 4 plus 8 always fits into a
> size_t.
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
>  source3/libsmb/cli_smb2_fnum.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/source3/libsmb/cli_smb2_fnum.c b/source3/libsmb/cli_smb2_fnum.c
> index 3537932c0d0..74f2f2ec4e4 100644
> --- a/source3/libsmb/cli_smb2_fnum.c
> +++ b/source3/libsmb/cli_smb2_fnum.c
> @@ -2943,10 +2943,6 @@ NTSTATUS cli_smb2_get_user_quota(struct cli_state *cli,
>  	sid_len = ndr_size_dom_sid(&pqt->sid, 0);
>  
>  	query.return_single = 1;
> -	if (sid_len < 0) {
> -		status = NT_STATUS_INVALID_PARAMETER;
> -		goto fail;
> -	}
>  
>  	info.next_entry_offset = 0;
>  	info.sid_length = sid_len;
> -- 
> 2.11.0
> 
> 
> From 849dd496d83076eb6d9d1a68b9a7e81c3414eab3 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Wed, 8 Aug 2018 10:14:26 +0200
> Subject: [PATCH 5/5] libsmb: Fix CID 1438243 Unchecked return value
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
>  source3/libsmb/cliquota.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/source3/libsmb/cliquota.c b/source3/libsmb/cliquota.c
> index 8efd2bbe38a..52f98eb9e8f 100644
> --- a/source3/libsmb/cliquota.c
> +++ b/source3/libsmb/cliquota.c
> @@ -649,7 +649,14 @@ NTSTATUS fill_quota_buffer(TALLOC_CTX *mem_ctx,
>  		/* pidl will align to 8 bytes due to 8 byte members*/
>  		/* Remember how much align padding we've used. */
>  		padding = qndr->offset;
> -		ndr_push_align(qndr, 8);
> +
> +		err = ndr_push_align(qndr, 8);
> +		if (!NDR_ERR_CODE_IS_SUCCESS(err)) {
> +			DBG_DEBUG("ndr_push_align returned %s\n",
> +				  ndr_map_error2string(err));
> +			return ndr_map_error2ntstatus(err);
> +		}
> +
>  		padding = qndr->offset - padding;
>  
>  		/*
> -- 
> 2.11.0
> 




More information about the samba-technical mailing list