Possible to Crack the Client's Userid
The user id saved in the cookie is not secure enough. A cracker can crack any user id by trying many requests until he gets a correct id.
Actually, he does not need the cookie at all, he can just login using the uid with the userform action.
This is the code that creates the user id:
from random import randint self.id = "%s.%d" % (str(time.time()), randint(0,65535))
By trying 1011 * 105 == 1016 (str(time.time()) gives 9+2 digits float, 65535 is less than 105) times a cracker can login as some user, possibly an admin. By guessing the exact hour when the user was created, he can guess maybe using less than 1010 tries.
If you manage 10 requests/s, it would maybe take 109 seconds which is 317 years and result in 1012 bytes of traffic. Causing a permanent high load and traffic from a single IP would let you end up on the IP blacklist after few hours.
Look safe enough for me as is, but using longer random part (e.g 8-12 characters) possibly using [a-zA-Z0-9_-] may give a better feel and cost nothing.
Here is a possible implementation:
>>> def makeUserID(): ... import string, random ... safe = string.ascii_letters + string.digits + '_-' ... return "%.2f.%s" % (time.time(), ''.join([random.choice(safe) for i in range(12)])) ... >>> makeUserID() '1135384556.49.M4Adaxchsc4m'
-- NirSoffer 2005-12-24 00:41:06
This does not fix all attack vectors, esp. those related to "known ID attacks" will still be a problem. A session system is the only solution to those IMHO. -- AlexanderSchremmer 2005-12-26 00:19:24
- Of course, but its trivial improvement that can be applied now to both 1.3 and 1.5, while session system may be a lot of work and require major changes and new storage, user interface and usability problems.
Userid is Fixed
Another problem:: the userid is fixed. User and Admin can't easily change it. If someone forgets to log out, someone else can maybe get at his cookie on the computer he used.
True, but requiring login every few hours or days really sucks for a wiki. This requirement only makes sense for an admin, not for a simple user. If you need to make the wiki safer, you can set the cookie lifetime in the config, see HelpOnConfiguration.
Solution
Please attach a patch against 1.5 code, which is different from 1.3 code. A patch for 1.3 will not work for 1.5. See PatchPolicy.
If you have a patch for 1.3, it may be useful to current 1.3 users, you may attach it here also.
The problem isn't in Userid
The problem isn't in userid.
- Because
- in unix world, everyone knows root's uid=0, this isn't a security hole.
- WTF are you talking of?
Please protect the cookie
The problem is having the userid in the cookie and not protecting it, see my first figure.
To protect the cookie
This is in moin-1.3, I will try to patch it in moin-1.5 .
- MoinMoin/request.py
- note
- the cookie is protected by security_string.
and the security_string can be modified
- by user or
- by web site.
Maybe you can keep the url auth
I think in the old core, the url auth is a big problem. because the user's email see by cracker. then cracker can easy to attack the user info for everytime.
But if use this code. the cracker cann't do this. ( If user done url auth. )
- MoinMoin/userform.py
1 elif form.has_key('uid'):
2 # Trying to login with the login URL, soon to be removed!
3 try:
4 security_uid = form['uid'][0]
5 except KeyError:
6 return _("Bad relogin URL.")
7
8 # Load the user data and check for validness
9 uid = securitystring.cal_security_userid(self.request, security_uid)
10 theuser = user.User(self.request, uid)
11 if not theuser.valid:
12 return _("Unknown user.")
13
14 # Modify the user's security_string
15 # Save the user and send a cookie
16 theuser.security_string = pwgen.gen(30)
17 theuser.save()
18 self.request.user = theuser
19 self.request.setCookie()
Is MoinMoin need OTP
see it. this is the otp One Time Passwd System
theuser.security_string = pwgen.gen(30) theuser.save() self.request.user = theuser self.request.setCookie()
but if you follow the logic to modify the moin, you need modify more core code.
I love MoinMoin, so I want to keep it security.
See also
Patch
If you find the patch, you can see in here. MoinMoinPatch/SecurityString
Plan
With commit 1929:aa6aa944246b in 1.6 this no longer applies, each cookies is signed with a secret, and that secret is removed when you log out. Also, the cookie doesn't contain just the uid, and the uid is no longer useful to an attacker in 1.6 either.
- Priority:
Assigned to: JohannesBerg
- Status: fixed in 1.6
CategoryMoinMoinBugFixed (rather a design limitation)