changeset 2630:f405012e67af

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.
author Thomas Waldmann <tw AT waldmann-edv DOT de>
date Sun, 20 Apr 2008 18:37:03 +0200
parents fa7e99b19a43
children b70e98cb53b8
files MoinMoin/userform.py docs/CHANGES
diffstat 2 files changed, 73 insertions(+), 65 deletions(-) [+]
line wrap: on
line diff
--- a/MoinMoin/userform.py	Sun Apr 20 18:08:43 2008 +0200
+++ b/MoinMoin/userform.py	Sun Apr 20 18:37:03 2008 +0200
@@ -10,6 +10,7 @@
 import time
 from MoinMoin import user, util, wikiutil
 from MoinMoin.widget import html
+from MoinMoin.auth import moin_session
 
 _debug = 0
 
@@ -95,37 +96,31 @@
             form.has_key('create_and_mail')):
             if self.request.request_method != 'POST':
                 return _("Use UserPreferences to change your settings or create an account.", formatted=False)
-            
+
             from MoinMoin.security.textcha import TextCha
             if not TextCha(self.request).check_answer_from_form():
                 return _('TextCha: Wrong answer! Go back and try again...', formatted=False)
 
             # Create user profile
-            if form.has_key('create'):
-                theuser = self.request.get_user_from_form()
-            else:
-                theuser = user.User(self.request, auth_method="request:152")
+            theuser = user.User(self.request, auth_method='new-user')
 
             # Require non-empty name
-            try:
-                theuser.name = form['name'][0]
-            except KeyError:
+            name = form.get('name', [''])[0]
+            if not name:
                 return _("Empty user name. Please enter a user name.", formatted=False)
 
             # Don't allow users with invalid names
-            if not user.isValidName(self.request, theuser.name):
+            if not user.isValidName(self.request, name):
                 return _("""Invalid user name {{{'%s'}}}.
 Name may contain any Unicode alpha numeric character, with optional one
-space between words. Group page name is not allowed.""", formatted=True, percent=True) % wikiutil.escape(theuser.name)
+space between words. Group page name is not allowed.""", formatted=True, percent=True) % wikiutil.escape(name)
 
-            # Is this an existing user trying to change information or a new user?
-            # Name required to be unique. Check if name belong to another user.
-            newuser = 1
-            if user.getUserId(self.request, theuser.name):
-                if theuser.name != self.request.user.name:
-                    return _("This user name already belongs to somebody else.", formatted=False)
-                else:
-                    newuser = 0
+            # new user
+            if user.getUserId(self.request, name): # a userid for new name exists
+                return _("This user name already belongs to somebody else.", formatted=False)
+
+            # all checks ok, assign name
+            theuser.name = name
 
             # try to get the password and pw repeat
             password = form.get('password', [''])[0]
@@ -135,14 +130,13 @@
             if password != password2:
                 return _("Passwords don't match!", formatted=False)
 
-            if newuser:
-                if not password:
-                    return _("Please specify a password!", formatted=False)
-                pw_checker = self.request.cfg.password_checker
-                if pw_checker:
-                    pw_error = pw_checker(theuser.name, password)
-                    if pw_error:
-                        return _("Password not acceptable: %s", formatted=False) % pw_error
+            if not password:
+                return _("Please specify a password!", formatted=False)
+            pw_checker = self.request.cfg.password_checker
+            if pw_checker:
+                pw_error = pw_checker(theuser.name, password)
+                if pw_error:
+                    return _("Password not acceptable: %s", formatted=False) % pw_error
 
             # Encode password
             if password and not password.startswith('{SHA}'):
@@ -154,21 +148,28 @@
 
             # try to get the (required) email
             email = wikiutil.clean_input(form.get('email', [''])[0])
-            theuser.email = email.strip()
-            if not theuser.email:
+            email = email.strip()
+
+            if not email:
                 return _("Please provide your email address. If you lose your"
                          " login information, you can get it by email.", formatted=False)
 
             # Email should be unique - see also MoinMoin/script/accounts/moin_usercheck.py
-            if theuser.email and self.request.cfg.user_email_unique:
-                if user.get_by_email_address(self.request, theuser.email):
+            if self.request.cfg.user_email_unique:
+                email_user = user.get_by_email_address(self.request, email)
+                if email_user:
                     return _("This email already belongs to somebody else.", formatted=False)
 
+            theuser.email = email
+
             # save data
             theuser.save()
             if form.has_key('create_and_mail'):
                 theuser.mailAccountData()
 
+            self.request.user = theuser
+            moin_session(self.request, user_obj=theuser) # establish session (saves having to do extra login)
+
             result = _("User account created! You can use this account to login now...", formatted=False)
             if _debug:
                 result = result + util.dumpFormData(form)
@@ -191,10 +192,11 @@
                     self.request.user = self.request._setuid_real_user
                     self.request._setuid_real_user = None
                 else:
+                    if self.request._setuid_real_user is None:
+                        self.request._setuid_real_user = self.request.user
                     theuser = user.User(self.request, uid)
                     theuser.disabled = None
                     self.request.session['setuid'] = uid
-                    self.request._setuid_real_user = self.request.user
                     # now continue as the other user
                     self.request.user = theuser
                 return  _("Use UserPreferences to change settings of the selected user account", formatted=False)
@@ -204,28 +206,31 @@
         if form.has_key('save'): # Save user profile
             if self.request.request_method != 'POST':
                 return _("Use UserPreferences to change your settings or create an account.", formatted=False)
-            theuser = self.request.get_user_from_form()
-
+            theuser = self.request.user                 # this is either just the currently logged in (normal) user OR
+                                                        # the (normal) user that the superuser su'd to
+            newuser = False
             if not 'name' in theuser.auth_attribs:
+                name = form.get('name', [theuser.name])[0]
                 # Require non-empty name
-                theuser.name = form.get('name', [theuser.name])[0]
-            if not theuser.name:
-                return _("Empty user name. Please enter a user name.", formatted=False)
+                if not name:
+                    return _("Empty user name. Please enter a user name.", formatted=False)
 
-            # Don't allow users with invalid names
-            if not user.isValidName(self.request, theuser.name):
-                return _("""Invalid user name {{{'%s'}}}.
+                # Don't allow users with invalid names
+                if not user.isValidName(self.request, name):
+                    return _("""Invalid user name {{{'%s'}}}.
 Name may contain any Unicode alpha numeric character, with optional one
-space between words. Group page name is not allowed.""", formatted=True, percent=True) % wikiutil.escape(theuser.name)
+space between words. Group page name is not allowed.""", formatted=True, percent=True) % wikiutil.escape(name)
 
-            # Is this an existing user trying to change information or a new user?
-            # Name required to be unique. Check if name belong to another user.
-            newuser = 1
-            if user.getUserId(self.request, theuser.name):
-                if theuser.name != self.request.user.name:
-                    return _("This user name already belongs to somebody else.", formatted=False)
-                else:
-                    newuser = 0
+                # Is this an existing user trying to change information or a new user?
+                # Name required to be unique. Check if name belongs to another user.
+                if name != theuser.name:
+                    # either new user or user wants to change his name (edited the name form field)
+                    if user.getUserId(self.request, name): # a userid for new name exists
+                        return _("This user name already belongs to somebody else.", formatted=False)
+                    newuser = not user.getUserId(self.request, theuser.name)
+
+                # all checks ok, assign name
+                theuser.name = name
 
             if not 'password' in theuser.auth_attribs:
                 # try to get the password and pw repeat                
@@ -253,25 +258,21 @@
                         # Should never happen
                         return "Can't encode password: %s" % str(err)
 
+            # try to get the (required) email
             if not 'email' in theuser.auth_attribs:
-                # try to get the email
                 email = wikiutil.clean_input(form.get('email', [theuser.email])[0])
-                theuser.email = email.strip()
+                email = email.strip()
+                if not email:
+                    return _("Please provide your email address. If you lose your"
+                             " login information, you can get it by email.", formatted=False)
 
-            # Require email
-            if not theuser.email:
-                return _("Please provide your email address. If you lose your"
-                         " login information, you can get it by email.", formatted=False)
+                # Email should be unique - see also MoinMoin/script/accounts/moin_usercheck.py
+                if email and self.request.cfg.user_email_unique:
+                    email_user = user.get_by_email_address(self.request, email)
+                    if email_user and email_user.id != theuser.id:
+                        return _("This email already belongs to somebody else.", formatted=False)
 
-            # Email should be unique - see also MoinMoin/script/accounts/moin_usercheck.py
-            if theuser.email and self.request.cfg.user_email_unique:
-                users = user.getUserList(self.request)
-                for uid in users:
-                    if uid == theuser.id:
-                        continue
-                    thisuser = user.User(self.request, uid, auth_method='userform:283')
-                    if thisuser.email == theuser.email:
-                        return _("This email already belongs to somebody else.", formatted=False)
+                theuser.email = email
 
             if not 'aliasname' in theuser.auth_attribs:
                 # aliasname
@@ -358,7 +359,6 @@
 
             # save data
             theuser.save()
-            self.request.user = theuser
 
             result = _("User preferences saved!", formatted=False)
             if _debug:
@@ -563,7 +563,7 @@
                         if key == 'password':
                             name = 'password1'
                         else:
-                            name = key                            
+                            name = key
                         self.make_row(_(label, formatted=False),
                                   [html.INPUT(type=type, size=length, name=name, value=getattr(self.request.user, key)), ' ', _(textafter, formatted=False), ])
 
--- a/docs/CHANGES	Sun Apr 20 18:08:43 2008 +0200
+++ b/docs/CHANGES	Sun Apr 20 18:37:03 2008 +0200
@@ -30,6 +30,10 @@
 
 Version 1.6.current:
   Fixes:
+    * Security fix: a check in the user form processing was not working as
+      expected, leading to a major ACL and superuser priviledge escalation
+      problem. If you use ACL entries other than "Known:" or "All:" and/or
+      a non-empty superuser list, you need to urgently install this upgrade.
     * Security fix: if acl_hierarchic=True was used (False is the default),
       ACL processing was wrong for some cases, see
       MoinMoinBugs/AclHierarchicPageAclSupercededByAclRightsAfter
@@ -55,6 +59,10 @@
   Other changes:
     * Added 'notes' to config.url_schemas, so you can use notes://notessrv/...
       to invoke your Lotus Notes client.
+    * After creating a new user profile via UserPreferences, you are logged
+      in with that user (no need to immediately enter the same name/password
+      again for logging in).
+
 
 Version 1.6.2:
   Fixes: