Item11501: Sync validation code from Release11

Validate that no registration field contains html markup. Uses simple
test for < or >.   Bypass testing of the password and password confirm
fields.

Still needs to be incorporated into BulkRegistration, but that is done
by Admin Users, so no risk.

Add a unit test.

git-svn-id: http://svn.foswiki.org/trunk@13913 0b4bb1d4-4e5a-0410-9cc4-b2b747904278

TopicUserMappingContrib/lib/Foswiki/Users/TopicUserMapping.pm

@@ -1500,6 +1500,29 @@ sub passwordError {
     return $this->{passwords}->error();
 }
 
+=begin TML
+
+---++ ObjectMethod validateRegistrationField($field, $value ) -> $string
+
+This method is called for every field submitted during registration.  It is also used
+to validate the username when adding a member to a group.
+
+Returns a string containing the sanitized registration field, or can throw an Error::Simple
+if the field contains illegal data to block the registration.
+
+returns the string unchanged if no issue found.
+
+=cut
+
+sub validateRegistrationField {
+
+    #my ($this, $field, $value) = @_;
+    my $this = shift;
+
+# For now just let Foswiki::UserMapping do the validation - nothing special needed.
+    return $this->SUPER::validateRegistrationField(@_);
+}
+
 # TODO: and probably flawed in light of multiple cUIDs mapping to one wikiname
 sub _cacheUser {
     my ( $this, $wikiname, $login ) = @_;

UnitTestContrib/test/unit/RegisterTests.pm

@@ -877,6 +877,64 @@ sub verify_rejectShortPassword {
     return;
 }
 
+# Register a user with invalid characters in a field - like < html
+sub verify_rejectEvilContent {
+    my $this = shift;
+    $Foswiki::cfg{Register}{NeedVerification} = 0;
+    $Foswiki::cfg{MinPasswordLength}          = 6;
+    $Foswiki::cfg{PasswordManager}            = 'Foswiki::Users::HtPasswdUser';
+    $Foswiki::cfg{Register}{AllowLoginName}   = 0;
+    my $query = Unit::Request->new(
+        {
+            'TopicName'    => ['UserRegistration'],
+            'Twk1Email'    => [ $this->{new_user_email} ],
+            'Twk1WikiName' => [ $this->{new_user_wikiname} ],
+            'Twk1Name'     => [ $this->{new_user_fullname} ],
+            'Twk0Comment'  => [''],
+
+            #'Twk1LoginName' => [ 'Bad@User' ],
+            'Twk1FirstName'    => [ $this->{new_user_fname} ],
+            'Twk1LastName'     => [ $this->{new_user_sname} ],
+            'Twk1Password'     => ['12345aaaaa'],
+            'Twk1Confirm'      => ['12345aaaaa'],
+            'Twk0Organization' => ['<script>Bad stuff</script>'],
+            'action'           => ['register'],
+        }
+    );
+
+    $query->path_info("/$this->{users_web}/UserRegistration");
+    $this->createNewFoswikiSession( $Foswiki::cfg{DefaultUserLogin}, $query );
+    $this->{session}->net->setMailHandler( \&FoswikiFnTestCase::sentMail );
+
+    try {
+        $this->captureWithKey( register => $REG_UI_FN, $this->{session} );
+    }
+    catch Foswiki::OopsException with {
+        my $e = shift;
+        $this->assert_str_equals( "attention", $e->{template},
+            $e->stringify() );
+        $this->assert_str_equals( "bad_password", $e->{def}, $e->stringify() );
+        $this->assert_equals( 0, scalar(@FoswikiFnTestCase::mails) );
+        @FoswikiFnTestCase::mails = ();
+    }
+    catch Foswiki::AccessControlException with {
+        my $e = shift;
+        $this->assert( 0, $e->stringify );
+    }
+    catch Error::Simple with {
+        my $e = shift;
+        $this->assert_equals(
+            'Invalid Organization',
+            substr( $e->stringify, 0, 20 )
+        );
+    }
+    otherwise {
+        $this->assert( 0, "expected an oops redirect" );
+    };
+
+    return;
+}
+
 # Register a user with a password which is too short
 sub verify_shortPassword {
     my $this = shift;

core/lib/Foswiki/UI/Register.pm

@@ -216,6 +216,7 @@ sub bulkRegister {
     #-- Process each row, generate a log as we go
     for ( my $n = 0 ; $n < scalar(@data) ; $n++ ) {
         my $row = $data[$n];
+
         $row->{webName} = $userweb;
 
         unless ( $row->{WikiName} ) {
@@ -275,6 +276,14 @@ sub _registerSingleBulkUser {
     }
 
     try {
+
+  #SMELL: Field Validations
+  # foreach my $field ( %$row ) {
+  #  - validate with Users::validateRegistrationField( $field, $row->{$field} );
+  #  - catch any errors to log
+  # Note, do this HERE and not in _validateRegistration.
+  # CGI registration validates earlier in _getDataFromQuery()
+
         _validateRegistration( $session, $row, 0 );
     }
     catch Foswiki::OopsException with {
@@ -380,7 +389,7 @@ sub _innerRegister {
     my ($session) = @_;
 
     my $query = $session->{request};
-    my $data = _getDataFromQuery( $query, $query->param() );
+    my $data = _getDataFromQuery( $session->{users}, $query, $query->param() );
 
     $data->{webName} = $session->{webName};
 
@@ -408,7 +417,7 @@ sub _requireVerification {
     my $topic = $session->{topicName};
     my $web   = $session->{webName};
 
-    my $data = _getDataFromQuery( $query, $query->param() );
+    my $data = _getDataFromQuery( $session->{users}, $query, $query->param() );
     my $oldName = $data->{WikiName};
     $data->{WikiName} =
       Foswiki::Sandbox::untaint( $data->{WikiName},
@@ -700,6 +709,7 @@ sub addUserToGroup {
           );
 
         try {
+            $u = $session->{users}->validateRegistrationField( 'username', $u );
             Foswiki::Func::addUserToGroup( $u, $groupName, $create );
             push( @succeeded, $u );
         }
@@ -825,7 +835,7 @@ sub _complete {
         _clearPendingRegistrationsForUser($code);
     }
     else {
-        $data = _getDataFromQuery( $query, $query->param() );
+        $data = _getDataFromQuery( $session->{users}, $query, $query->param() );
         $data->{webName} = $web;
     }
 
@@ -1545,6 +1555,7 @@ sub _loadPendingRegistration {
 }
 
 sub _getDataFromQuery {
+    my $users = shift;
     my $query = shift;
 
     # get all parameters from the form
@@ -1560,14 +1571,19 @@ sub _getDataFromQuery {
             # deal with multivalue fields like checkboxen
             my $value = join( ',', @values );
 
-            # Note: field values are unvalidated (and therefore tainted).
-            # This is because the registration code does not have enough
-            # information to validate the data - for example, it cannot
-            # know what the user mapper considers to be a valid login name.
-            # It is the responsibility of the implementation code to untaint
-            # these data before they are used in dangerous ways.
-            # DO NOT UNTAINT THESE DATA HERE!
-            $data->{$name} = $value;
+            try {
+                $data->{$name} = $users->validateRegistrationField( $name, $value );
+            }
+            catch Error::Simple with {
+                my $e = shift;
+                throw Foswiki::OopsException(
+                    'attention',
+                    #web    => $data->{webName},
+                    #topic  => $session->{topicName},
+                    def    => 'invalid_field',
+                    params => [ $name ]
+                );
+            };
             push(
                 @{ $data->{form} },
                 {

core/lib/Foswiki/UserMapping.pm

@@ -541,6 +541,37 @@ sub passwordError {
     return;
 }
 
+=begin TML
+
+---++ ObjectMethod validateRegistrationField($field, $value ) -> $string
+
+Returns a string containing the sanitized registration field, or can throw an oops
+if the field contains illegal data to block the registration.
+
+returns the string unchanged if no issue found.
+
+=cut
+
+sub validateRegistrationField {
+
+    #my ($this, $field, $value) = @_;
+
+    # Filter username per the login validation rules.
+    if ( lc( $_[1] ) eq 'username'
+        && !( $_[2] =~ m/$Foswiki::cfg{LoginNameFilterIn}/ ) )
+    {
+        throw Error::Simple("Invalid username");
+    }
+
+    # Don't check contents of password - it's never displayed.
+    return $_[2] if ( lc( $_[1] ) eq 'password' || lc( $_[1] ) eq 'confirm' );
+
+    # Don't allow html markup in any other fields.
+    throw Error::Simple("Invalid $_[1]") if ( $_[2] =~ m/[<>]+/ );
+
+    return $_[2];
+}
+
 1;
 __END__
 Foswiki - The Free and Open Source Wiki, http://foswiki.org/

core/lib/Foswiki/Users.pm

@@ -237,6 +237,19 @@ sub supportsRegistration {
 
 =begin TML
 
+---++ ObjectMethod validateRegistrationField ( $field, $value ) -> text
+
+Return the registration formfield sanitized by the mapper,  or oops thrown to block the registration.
+
+=cut
+
+sub validateRegistrationField {
+    my ($this) = shift;
+    return $this->{mapping}->validateRegistrationField(@_);
+}
+
+=begin TML
+
 ---++ ObjectMethod initialiseUser ($login) -> $cUID
 
 Given a login (which must have been authenticated) determine the cUID that

core/templates/messages.tmpl

@@ -243,6 +243,12 @@
 ---+++ %MAKETEXT{"Missing Fields"}%
 =<font color="red">**</font>= %PARAM1% %MAKETEXT{"fields are required."}%
 
+%MAKETEXT{"Please go back in your browser and try again."}%
+%TMPL:END%
+%TMPL:DEF{"invalid_field"}%
+---+++ %MAKETEXT{"Invalid Input"}%
+=<font color="red">**</font>= %MAKETEXT{"The value supplied for =[_1]= has been rejected." args="%PARAM1%"}%
+
 %MAKETEXT{"Please go back in your browser and try again."}%
 %TMPL:END%
 %TMPL:DEF{"password_mismatch"}%