[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