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

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