[Samba] rpcclient netshareenum 502 causes SEGV

Jeremy Allison jra at samba.org
Mon Aug 19 18:20:24 MDT 2013


On Mon, Aug 19, 2013 at 06:21:02PM -0400, pisymbol . wrote:
> Hello:
> 
> I have a Windows 2003 Server that is causing rpcclient to SEGV via the
> following command:
> 
> $ rpcclient -U Administrator%foobar -c 'netshareenum 502' <server>
> ...
> type: 0x6269: SEC_DESC_OWNER_DEFAULTED SEC_DESC_DACL_DEFAULTED
> SEC_DESC_SACL_DEFAULTED SEC_DESC_DACL_TRUSTED
> SEC_DESC_SACL_AUTO_INHERIT_REQ SEC_DESC_SACL_PROTECTED
> SEC_DESC_RM_CONTROL_VALID
> SACL
> Segmentation fault (core dumped)
> 
> I did a little poking and it seems that the issue is here:
> 
> source3/rpcclient/cmd_srvsvc.c:
> 384     case
> 502:
> 
> 385         for (i = 0; i <
> totalentries;i++)
> 
> 386
> display_share_info_502(&info_ctr.ctr.ctr502->array[i]);
> 387         break;
> 
> Sorry for the formatting. But the NDR code yanks out 35 SHARE_INFO_502* *
> entries* *but the array size NDR code calculates only 34. Since
> "totalentries" is one entry too big, it causes rpcclient to go past the end
> of the ctr502 array and SEGV.
> 
> See here:
> 
> (gdb) p *info_ctr.ctr.ctr502
> $9 = {
>   count = 34,
>   array = 0x67a140
> }
> (gdb) p totalentries
> $10 = 35
> 
> Commit history shows that when the specific enum shares got unionized this
> loop changed to use "totalentries" intsead of "ctr.num_entries," which
> without looking into it might have been equivalent to "count."
> 
> It would seem to me that "totalentries" really has to be bounds checked
> here else you can fall into this trap.
> 
> I know this is ugly, but couldn't something be done like
> offsetof(ctr.share.infoXX, count) to verify that that the array size and
> total entries match. Or perhaps even better check this bounds condition
> during the NDR pull out unmarshalling code? (that is what I would vote for
> since it puts less of a burden on the callee but there may be cases where
> knowing the total entries vs what is in the array is useful, not sure...).
> 
> I am by no means a Samba expert but any insight into this issue would be
> greatly appreciated.

Actually I think that "totalentries" is just the wrong thing
to use here.

Can you try the following patch to see if it fixes the problem ?

Jeremy.
-------------- next part --------------
diff --git a/source3/rpcclient/cmd_srvsvc.c b/source3/rpcclient/cmd_srvsvc.c
index 0d67639..e5fa065 100644
--- a/source3/rpcclient/cmd_srvsvc.c
+++ b/source3/rpcclient/cmd_srvsvc.c
@@ -273,6 +273,7 @@ static WERROR cmd_srvsvc_net_share_enum_int(struct rpc_pipe_client *cli,
 	WERROR result;
 	NTSTATUS status;
 	uint32_t totalentries = 0;
+	uint32_t count = 0;
 	uint32_t resume_handle = 0;
 	uint32_t *resume_handle_p = NULL;
 	uint32 preferred_len = 0xffffffff, i;
@@ -374,15 +375,18 @@ static WERROR cmd_srvsvc_net_share_enum_int(struct rpc_pipe_client *cli,
 
 	switch (info_level) {
 	case 1:
-		for (i = 0; i < totalentries; i++)
+		count = info_ctr.ctr.ctr1->count;
+		for (i = 0; i < count; i++)
 			display_share_info_1(&info_ctr.ctr.ctr1->array[i]);
 		break;
 	case 2:
-		for (i = 0; i < totalentries; i++)
+		count = info_ctr.ctr.ctr2->count;
+		for (i = 0; i < count; i++)
 			display_share_info_2(&info_ctr.ctr.ctr2->array[i]);
 		break;
 	case 502:
-		for (i = 0; i < totalentries; i++)
+		count = info_ctr.ctr.ctr502->count;
+		for (i = 0; i < count; i++)
 			display_share_info_502(&info_ctr.ctr.ctr502->array[i]);
 		break;
 	default:


More information about the samba mailing list