Bug 2300 - potential memory corruption in _samr_delete_dom_group
Summary: potential memory corruption in _samr_delete_dom_group
Status: RESOLVED LATER
Alias: None
Product: Samba 3.0
Classification: Unclassified
Component: User/Group Accounts (show other bugs)
Version: 3.0.10
Hardware: All All
: P3 critical
Target Milestone: none
Assignee: Gerald (Jerry) Carter
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-02-02 19:18 UTC by James Peach
Modified: 2005-02-07 07:36 UTC (History)
0 users

See Also:


Attachments
take group name from SID lookup (569 bytes, patch)
2005-02-02 19:20 UTC, James Peach
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description James Peach 2005-02-02 19:18:02 UTC
In rpc_server/srv_samr_nt.c:_samr_delete_dom_group(), the grp variable is
uninitialised when it is passed to smb_delete_group().

Bad things will happen if either lp_delgroup_script or
lp_winbind_enable_local_accounts is set.
Comment 1 James Peach 2005-02-02 19:20:14 UTC
Created attachment 939 [details]
take group name from SID lookup

I think what is supposed to happen is for the group name to come from
sid_to_local_dom_grp_name().
Comment 2 Gerald (Jerry) Carter 2005-02-02 21:27:58 UTC
how can grp it be uninitialized when we just called 
grp=getgrpgid() a few lines before.   WHat am I not 
seeing here?
Comment 3 James Peach 2005-02-02 21:55:51 UTC
(In reply to comment #2)
> how can grp it be uninitialized when we just called 
> grp=getgrpgid() a few lines before.   WHat am I not 
> seeing here?

I don't see this call anywhere in _samr_delete_dom_group(). I'm looking
at SVN revision 5184.
Comment 4 Volker Lendecke 2005-02-03 02:34:06 UTC
James, you're looking at trunk, right? srv_samr_nt.c seems wildly different
between 3_0 and trunk, and this needs merging. I'm closing this bug with
"later", when the merge is done.

Volker
Comment 5 Gerald (Jerry) Carter 2005-02-03 05:51:53 UTC
ok.  That pretty much settles it then.  I had wondered
if trunk had more work on the group stuff.  I'm going to 
merge since code from 3.0 since trunk is obviously wrong.
Comment 6 James Peach 2005-02-03 15:22:09 UTC
(In reply to comment #4)
> James, you're looking at trunk, right? srv_samr_nt.c seems wildly different
> between 3_0 and trunk, and this needs merging. I'm closing this bug with
> "later", when the merge is done.

Yes, I'm looking at trunk. Doesn't trunk get merged into 3_0 for releases?

Comment 7 Volker Lendecke 2005-02-03 15:55:21 UTC
James,

trunk/ is not necessarily merged into trunk, it is more like a testbed for
ideas. For example I've started the third attempt to get winbind non-blocking,
all three with completely different ideas. Or, lately stuff that mimir has done
has been revoked again. If you want to get something quality-checked by the very
valuable IRIX compiler warnings, please look at the branch SAMBA_3_0. This is
the one we really care about.

Thanks,

Volker
Comment 8 Gerald (Jerry) Carter 2005-02-03 16:39:55 UTC
no.  trunk uis basically 3.1.  The SAMBA_3_0 is teh stable 
development tree which gets merged to the SAMBA_3_0_RELEASE 
tree for release.
Comment 9 James Peach 2005-02-03 18:16:26 UTC
(In reply to comment #8)
> no.  trunk uis basically 3.1.  The SAMBA_3_0 is teh stable 
> development tree which gets merged to the SAMBA_3_0_RELEASE 
> tree for release.

I see .. thanks for setting me straight ..
Comment 10 Gerald (Jerry) Carter 2005-02-07 07:36:51 UTC
originally reported against 3.0.11pre1.  Moving back to version to 3.0.10 
to remove preX and rcX versions.