major security fix: a check in the userform processing was not working as expected and could be abused for ACL and superuser priviledge escalation. New: establish a session after user creation, no need to immediately enter same name/password again.
1.1 --- a/MoinMoin/userform.py Sun Apr 20 18:08:43 2008 +0200
1.2 +++ b/MoinMoin/userform.py Sun Apr 20 18:37:03 2008 +0200
1.3 @@ -10,6 +10,7 @@
1.4 import time
1.5 from MoinMoin import user, util, wikiutil
1.6 from MoinMoin.widget import html
1.7 +from MoinMoin.auth import moin_session
1.8
1.9 _debug = 0
1.10
1.11 @@ -95,37 +96,31 @@
1.12 form.has_key('create_and_mail')):
1.13 if self.request.request_method != 'POST':
1.14 return _("Use UserPreferences to change your settings or create an account.", formatted=False)
1.15 -
1.16 +
1.17 from MoinMoin.security.textcha import TextCha
1.18 if not TextCha(self.request).check_answer_from_form():
1.19 return _('TextCha: Wrong answer! Go back and try again...', formatted=False)
1.20
1.21 # Create user profile
1.22 - if form.has_key('create'):
1.23 - theuser = self.request.get_user_from_form()
1.24 - else:
1.25 - theuser = user.User(self.request, auth_method="request:152")
1.26 + theuser = user.User(self.request, auth_method='new-user')
1.27
1.28 # Require non-empty name
1.29 - try:
1.30 - theuser.name = form['name'][0]
1.31 - except KeyError:
1.32 + name = form.get('name', [''])[0]
1.33 + if not name:
1.34 return _("Empty user name. Please enter a user name.", formatted=False)
1.35
1.36 # Don't allow users with invalid names
1.37 - if not user.isValidName(self.request, theuser.name):
1.38 + if not user.isValidName(self.request, name):
1.39 return _("""Invalid user name {{{'%s'}}}.
1.40 Name may contain any Unicode alpha numeric character, with optional one
1.41 -space between words. Group page name is not allowed.""", formatted=True, percent=True) % wikiutil.escape(theuser.name)
1.42 +space between words. Group page name is not allowed.""", formatted=True, percent=True) % wikiutil.escape(name)
1.43
1.44 - # Is this an existing user trying to change information or a new user?
1.45 - # Name required to be unique. Check if name belong to another user.
1.46 - newuser = 1
1.47 - if user.getUserId(self.request, theuser.name):
1.48 - if theuser.name != self.request.user.name:
1.49 - return _("This user name already belongs to somebody else.", formatted=False)
1.50 - else:
1.51 - newuser = 0
1.52 + # new user
1.53 + if user.getUserId(self.request, name): # a userid for new name exists
1.54 + return _("This user name already belongs to somebody else.", formatted=False)
1.55 +
1.56 + # all checks ok, assign name
1.57 + theuser.name = name
1.58
1.59 # try to get the password and pw repeat
1.60 password = form.get('password', [''])[0]
1.61 @@ -135,14 +130,13 @@
1.62 if password != password2:
1.63 return _("Passwords don't match!", formatted=False)
1.64
1.65 - if newuser:
1.66 - if not password:
1.67 - return _("Please specify a password!", formatted=False)
1.68 - pw_checker = self.request.cfg.password_checker
1.69 - if pw_checker:
1.70 - pw_error = pw_checker(theuser.name, password)
1.71 - if pw_error:
1.72 - return _("Password not acceptable: %s", formatted=False) % pw_error
1.73 + if not password:
1.74 + return _("Please specify a password!", formatted=False)
1.75 + pw_checker = self.request.cfg.password_checker
1.76 + if pw_checker:
1.77 + pw_error = pw_checker(theuser.name, password)
1.78 + if pw_error:
1.79 + return _("Password not acceptable: %s", formatted=False) % pw_error
1.80
1.81 # Encode password
1.82 if password and not password.startswith('{SHA}'):
1.83 @@ -154,21 +148,28 @@
1.84
1.85 # try to get the (required) email
1.86 email = wikiutil.clean_input(form.get('email', [''])[0])
1.87 - theuser.email = email.strip()
1.88 - if not theuser.email:
1.89 + email = email.strip()
1.90 +
1.91 + if not email:
1.92 return _("Please provide your email address. If you lose your"
1.93 " login information, you can get it by email.", formatted=False)
1.94
1.95 # Email should be unique - see also MoinMoin/script/accounts/moin_usercheck.py
1.96 - if theuser.email and self.request.cfg.user_email_unique:
1.97 - if user.get_by_email_address(self.request, theuser.email):
1.98 + if self.request.cfg.user_email_unique:
1.99 + email_user = user.get_by_email_address(self.request, email)
1.100 + if email_user:
1.101 return _("This email already belongs to somebody else.", formatted=False)
1.102
1.103 + theuser.email = email
1.104 +
1.105 # save data
1.106 theuser.save()
1.107 if form.has_key('create_and_mail'):
1.108 theuser.mailAccountData()
1.109
1.110 + self.request.user = theuser
1.111 + moin_session(self.request, user_obj=theuser) # establish session (saves having to do extra login)
1.112 +
1.113 result = _("User account created! You can use this account to login now...", formatted=False)
1.114 if _debug:
1.115 result = result + util.dumpFormData(form)
1.116 @@ -191,10 +192,11 @@
1.117 self.request.user = self.request._setuid_real_user
1.118 self.request._setuid_real_user = None
1.119 else:
1.120 + if self.request._setuid_real_user is None:
1.121 + self.request._setuid_real_user = self.request.user
1.122 theuser = user.User(self.request, uid)
1.123 theuser.disabled = None
1.124 self.request.session['setuid'] = uid
1.125 - self.request._setuid_real_user = self.request.user
1.126 # now continue as the other user
1.127 self.request.user = theuser
1.128 return _("Use UserPreferences to change settings of the selected user account", formatted=False)
1.129 @@ -204,28 +206,31 @@
1.130 if form.has_key('save'): # Save user profile
1.131 if self.request.request_method != 'POST':
1.132 return _("Use UserPreferences to change your settings or create an account.", formatted=False)
1.133 - theuser = self.request.get_user_from_form()
1.134 + theuser = self.request.user # this is either just the currently logged in (normal) user OR
1.135 + # the (normal) user that the superuser su'd to
1.136 + newuser = False
1.137 + if not 'name' in theuser.auth_attribs:
1.138 + name = form.get('name', [theuser.name])[0]
1.139 + # Require non-empty name
1.140 + if not name:
1.141 + return _("Empty user name. Please enter a user name.", formatted=False)
1.142
1.143 - if not 'name' in theuser.auth_attribs:
1.144 - # Require non-empty name
1.145 - theuser.name = form.get('name', [theuser.name])[0]
1.146 - if not theuser.name:
1.147 - return _("Empty user name. Please enter a user name.", formatted=False)
1.148 + # Don't allow users with invalid names
1.149 + if not user.isValidName(self.request, name):
1.150 + return _("""Invalid user name {{{'%s'}}}.
1.151 +Name may contain any Unicode alpha numeric character, with optional one
1.152 +space between words. Group page name is not allowed.""", formatted=True, percent=True) % wikiutil.escape(name)
1.153
1.154 - # Don't allow users with invalid names
1.155 - if not user.isValidName(self.request, theuser.name):
1.156 - return _("""Invalid user name {{{'%s'}}}.
1.157 -Name may contain any Unicode alpha numeric character, with optional one
1.158 -space between words. Group page name is not allowed.""", formatted=True, percent=True) % wikiutil.escape(theuser.name)
1.159 + # Is this an existing user trying to change information or a new user?
1.160 + # Name required to be unique. Check if name belongs to another user.
1.161 + if name != theuser.name:
1.162 + # either new user or user wants to change his name (edited the name form field)
1.163 + if user.getUserId(self.request, name): # a userid for new name exists
1.164 + return _("This user name already belongs to somebody else.", formatted=False)
1.165 + newuser = not user.getUserId(self.request, theuser.name)
1.166
1.167 - # Is this an existing user trying to change information or a new user?
1.168 - # Name required to be unique. Check if name belong to another user.
1.169 - newuser = 1
1.170 - if user.getUserId(self.request, theuser.name):
1.171 - if theuser.name != self.request.user.name:
1.172 - return _("This user name already belongs to somebody else.", formatted=False)
1.173 - else:
1.174 - newuser = 0
1.175 + # all checks ok, assign name
1.176 + theuser.name = name
1.177
1.178 if not 'password' in theuser.auth_attribs:
1.179 # try to get the password and pw repeat
1.180 @@ -253,25 +258,21 @@
1.181 # Should never happen
1.182 return "Can't encode password: %s" % str(err)
1.183
1.184 + # try to get the (required) email
1.185 if not 'email' in theuser.auth_attribs:
1.186 - # try to get the email
1.187 email = wikiutil.clean_input(form.get('email', [theuser.email])[0])
1.188 - theuser.email = email.strip()
1.189 + email = email.strip()
1.190 + if not email:
1.191 + return _("Please provide your email address. If you lose your"
1.192 + " login information, you can get it by email.", formatted=False)
1.193
1.194 - # Require email
1.195 - if not theuser.email:
1.196 - return _("Please provide your email address. If you lose your"
1.197 - " login information, you can get it by email.", formatted=False)
1.198 + # Email should be unique - see also MoinMoin/script/accounts/moin_usercheck.py
1.199 + if email and self.request.cfg.user_email_unique:
1.200 + email_user = user.get_by_email_address(self.request, email)
1.201 + if email_user and email_user.id != theuser.id:
1.202 + return _("This email already belongs to somebody else.", formatted=False)
1.203
1.204 - # Email should be unique - see also MoinMoin/script/accounts/moin_usercheck.py
1.205 - if theuser.email and self.request.cfg.user_email_unique:
1.206 - users = user.getUserList(self.request)
1.207 - for uid in users:
1.208 - if uid == theuser.id:
1.209 - continue
1.210 - thisuser = user.User(self.request, uid, auth_method='userform:283')
1.211 - if thisuser.email == theuser.email:
1.212 - return _("This email already belongs to somebody else.", formatted=False)
1.213 + theuser.email = email
1.214
1.215 if not 'aliasname' in theuser.auth_attribs:
1.216 # aliasname
1.217 @@ -358,7 +359,6 @@
1.218
1.219 # save data
1.220 theuser.save()
1.221 - self.request.user = theuser
1.222
1.223 result = _("User preferences saved!", formatted=False)
1.224 if _debug:
1.225 @@ -563,7 +563,7 @@
1.226 if key == 'password':
1.227 name = 'password1'
1.228 else:
1.229 - name = key
1.230 + name = key
1.231 self.make_row(_(label, formatted=False),
1.232 [html.INPUT(type=type, size=length, name=name, value=getattr(self.request.user, key)), ' ', _(textafter, formatted=False), ])
1.233
2.1 --- a/docs/CHANGES Sun Apr 20 18:08:43 2008 +0200
2.2 +++ b/docs/CHANGES Sun Apr 20 18:37:03 2008 +0200
2.3 @@ -30,6 +30,10 @@
2.4
2.5 Version 1.6.current:
2.6 Fixes:
2.7 + * Security fix: a check in the user form processing was not working as
2.8 + expected, leading to a major ACL and superuser priviledge escalation
2.9 + problem. If you use ACL entries other than "Known:" or "All:" and/or
2.10 + a non-empty superuser list, you need to urgently install this upgrade.
2.11 * Security fix: if acl_hierarchic=True was used (False is the default),
2.12 ACL processing was wrong for some cases, see
2.13 MoinMoinBugs/AclHierarchicPageAclSupercededByAclRightsAfter
2.14 @@ -55,6 +59,10 @@
2.15 Other changes:
2.16 * Added 'notes' to config.url_schemas, so you can use notes://notessrv/...
2.17 to invoke your Lotus Notes client.
2.18 + * After creating a new user profile via UserPreferences, you are logged
2.19 + in with that user (no need to immediately enter the same name/password
2.20 + again for logging in).
2.21 +
2.22
2.23 Version 1.6.2:
2.24 Fixes: