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

4.6 CVS tip (haven't checked HEAD)

With a freshly installed system, group memberships look like this:

openacs4=# select * from groups;
 group_id |    group_name    | join_policy
----------+------------------+-------------
       -1 | The Public       | open
       -2 | Registered Users | open
(2 rows)
                                                                                
openacs4=# select * from group_member_map;
 group_id | member_id | rel_id | container_id |    rel_type
----------+-----------+--------+--------------+----------------
       -1 |         0 |      5 |           -1 | membership_rel
       -2 |       313 |    314 |           -2 | membership_rel
       -1 |       313 |    314 |           -2 | membership_rel
(3 rows)
                                                                               

Note that ID 0 is only a member of The Public (group -1). Now, without logging in, hit /register/user-join?group_id=-2

group memberships now look like this:

openacs4=# select * from group_member_map;
 group_id | member_id | rel_id | container_id |    rel_type
----------+-----------+--------+--------------+----------------
       -1 |         0 |      5 |           -1 | membership_rel
       -2 |       313 |    314 |           -2 | membership_rel
       -1 |       313 |    314 |           -2 | membership_rel
       -2 |         0 |    315 |           -2 | membership_rel
       -1 |         0 |    315 |           -2 | membership_rel

Now note that ID 0 is a member of Registered Users (-2). I'm not familiar with the ramifications of this, but I think the unregistered user should not become registered.

Can someone in the know say how bad this is?

Randy

Not as bad as you might think because in 4.6.3, our latest release version, 0 isn't really a user just a party, so queries expecting a user will normally return zero rows when they join to that table.

However it does mean that random visitors can probably add content here and there.  And can see content you'd normally only allow registered users to see, like user info pages with e-mail - a spammer might find this useful if they knew about the hole.

Should be easy to fix...

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);

Don, I'll test when I get a chance later this evening.

Randy

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.

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