Legal Security issue - hash could be stolen and used to access user account

Hi folks,

first thanks for your work. Please find attached my changes to enforce security of this module. If there is an existing user account base or the T&C will be changed, that the existing users have to re-apply to the new T&C then there are some security problems within your code.

You create a hash and uid like my.sytem/legal_accept/uid/hash.

If someone other than the user will become access to such a link, the account becomes a huge risc, because everyone who will use this link will be allowed to system as the starting user account. This happens because it is only checked that the hash and uid is a valid pair, but not that the initiator of the link is allowed to use that link and account. user_load is fired without any security checks.

I have made some thoughts to add things like ip-address, cookie, ... but I think everything will still not be secure. So I have change the validation in that way, that a user has to enter its full account and credential data to the acceptance form in order to accept new T&C. So its always secure.

I also had made a "user friendly option" to pre-enter users name already to login form, but then there is an new security problem, because unauthorized people could link users uid to their users name. So I have removed this option afterall and left user name in form empty.

Another unwished behavior I removed was, that you always moved to /user/uid page after acceptance of new T&C regardless were there user would go to. For example someone would add a new comment, he has to login, then must accept new T&C and then moved to his users page. With my changes destination will always be handled after acceptance.

I hope you agree to my changes, I have made also some small design changes, you will get it out from my patch and code.

The attached file includes my complete already patched legal.module as well as an unified patch between mine and source 5.x-1.6 release.

Tom

<--break-->