Forum OpenACS Q&A: Re: Security Issue: User ID 0 Becomes Member of Registered Users

Collapse
3: Re: Security Issue: User ID 0 Becomes Member of Registered Users (response to 1)
Posted by Don Baccus on 08/06/03 04:44 PM
Talli - you been reading code recently, or did I just accidently post as you? :)

I've fixed this by making the join policy for "The Public" and "Registered Users" closed rather than open.  The user registration ignores these constraints, using the registration option parameters instead so new users can still register.

You can change your existing site the same way my new upgrade script works:

update groups set join_policy = 'closed' where group_id in (-1,-2);

Collapse
5: Re: Security Issue: User ID 0 Becomes Member of Registered Users (response to 3)
Posted by Randy O'Meara on 08/07/03 04:06 AM
Don - your fix did not solve the problem.

After dropping and reinstalling, I applied: 

    update groups set join_policy = 'closed' where group_id in (-1,-2);
and then hit user-join as above. The result is identical to that shown above.

BTW, I started with a fresh CVS checkout from 4.6 tip. I checked after initial install and the join_policy on -1 and -2 was 'open', so it looks like your fix was not applied to 4.6 CVS.

Collapse
6: Re: Security Issue: User ID 0 Becomes Member of Registered Users (response to 5)
Posted by Randy O'Meara on 08/07/03 07:15 AM
Bug filed  at https://openacs.org/bugtracker/openacs/bug?bug_number=724.

Don - your fix should have solved the problem. There are (actually were, if my patch is accepted and applied) several problems with user-join.tcl, the worst being that the code was using ad_complain outside of ad_page_contract's -validate block which was doing nothing at all. It should have used ad_return_complaint and then return(ed) to the caller, and my patch handles this issue. There were checks for 'closed' groups and segments but the code wasn't handling returning any errors as I stated above.

Another issue was that user-join was not requiring a registered user so the unregistered user could have been added to any group with a non-closed join_policy. I fixed this be requiring a registered user for the request.

I also added (slightly) more descriptive text when an error is reported with ad_return_complaint, and I added a "return to previous page" link if return_url is contained in the query vars.

Lastly, I cleaned up the on_error block by checking the error message for a unique constraint violation that would indicate that the user is requesting membership in a group to which she already belongs. This condition is displayed along with the "return to previous..." link.

I think this hole is quite serious. That's why I took the time to fix it and generate a patch against today's 4.6 CVS tip. I hope this can be applied soon, and that site owners are aware that this hole exists.

Randy