Fix wrong handling of utf8 that could lead to server crashes
Change-Id: I0f21d61ccfa889a98cc6602d3d3ce3163a5c32b8
diff --git a/Changes b/Changes
index 8f3d0cb..3351bc5 100755
--- a/Changes
+++ b/Changes
@@ -43,6 +43,8 @@
- Support default values for state.
- Introduce state manager (#119).
- Support default value for plugin toggle embeddings.
+ - Fix wrong handling of utf8 input in login that can
+ lead to server crash.
0.42 2021-06-18
- Added GitHub based CI for perl.
diff --git a/lib/Kalamar/Plugin/Auth.pm b/lib/Kalamar/Plugin/Auth.pm
index df6302d..011dae1 100644
--- a/lib/Kalamar/Plugin/Auth.pm
+++ b/lib/Kalamar/Plugin/Auth.pm
@@ -3,7 +3,8 @@
use File::Basename 'dirname';
use File::Spec::Functions qw/catdir/;
use Mojo::ByteStream 'b';
-use Mojo::Util 'deprecated';
+use Mojo::Util qw!deprecated b64_encode encode!;
+use Encode 'is_utf8';
# This is a plugin to deal with the Kustvakt OAuth server.
# It establishes both the JWT as well as the OAuth password
@@ -65,6 +66,7 @@
logoutFail => 'Abmeldung fehlgeschlagen',
authenticationFail => 'Nicht authentifiziert',
csrfFail => 'Fehlerhafter CSRF Token',
+ invalidChar => 'Ungültiges Zeichen in Anfrage',
openRedirectFail => 'Weiterleitungsfehler',
tokenExpired => 'Zugriffstoken abgelaufen',
tokenInvalid => 'Zugriffstoken ungültig',
@@ -111,6 +113,7 @@
logoutFail => 'Logout failed',
authenticationFail => 'Not authenticated',
csrfFail => 'Bad CSRF token',
+ invalidChar => 'Invalid character in request',
openRedirectFail => 'Redirect failure',
tokenExpired => 'Access token expired',
tokenInvalid => 'Access token invalid',
@@ -608,7 +611,13 @@
$v->csrf_protect;
$v->optional('fwd')->closed_redirect;
- my $user = $v->param('handle');
+ my $user = check_decode($v->param('handle'));
+ unless ($user) {
+ $c->notify(error => $c->loc('Auth_invalidChar'));
+ $c->param(handle_or_email => '');
+ return $c->relative_redirect_to('index');
+ };
+
my $fwd = $v->param('fwd');
# Set flash for redirect
@@ -1304,7 +1313,13 @@
$v->csrf_protect;
$v->optional('fwd')->closed_redirect;
- my $user = $v->param('handle');
+ my $user = check_decode($v->param('handle'));
+ unless ($user) {
+ $c->notify(error => $c->loc('Auth_invalidChar'));
+ $c->param(handle_or_email => '');
+ return $c->relative_redirect_to('index');
+ };
+
my $fwd = $v->param('fwd');
# Set flash for redirect
@@ -1487,6 +1502,17 @@
};
+sub check_decode {
+ no warnings 'uninitialized';
+ my $str = shift;
+ my $str2 = is_utf8($str) ? b($str)->decode : $str;
+ if (defined($str2) && $str2 && length($str2) > 1) {
+ return $str2
+ };
+ return;
+};
+
+
1;
diff --git a/lib/Kalamar/Plugin/Auth/templates/partial/auth/login.html.ep b/lib/Kalamar/Plugin/Auth/templates/partial/auth/login.html.ep
index 61a73a5..6be3c5c 100644
--- a/lib/Kalamar/Plugin/Auth/templates/partial/auth/login.html.ep
+++ b/lib/Kalamar/Plugin/Auth/templates/partial/auth/login.html.ep
@@ -7,7 +7,7 @@
% param(handle => flash('handle_or_email'));
% };
<fieldset>
- %= form_for 'login', class => 'login', begin
+ %= form_for 'login', 'accept-charset' => 'utf-8', class => 'login', begin
<legend><span><%= loc 'login' %></span></legend>
%= csrf_field
%= text_field 'handle', placeholder => loc('username')
diff --git a/t/plugin/auth-oauth.t b/t/plugin/auth-oauth.t
index cfcfc87..635df8c 100644
--- a/t/plugin/auth-oauth.t
+++ b/t/plugin/auth-oauth.t
@@ -1,5 +1,6 @@
use Mojo::Base -strict;
use Test::More;
+use Mojo::ByteStream 'b';
use Test::Mojo::WithRoles 'Session';
use Mojo::File qw/path/;
use Data::Dumper;
@@ -432,9 +433,30 @@
is(defined $err ? $err->text : '', '');
-# Login:
+# This should fail
+my $wide_char_login = "\x{61}\x{E5}\x{61}"; # "\x{443}\x{434}";
$t->post_ok('/user/login' => form => {
- handle => 'test',
+ handle => $wide_char_login,
+ pwd => 'pass',
+ csrf_token => $csrf,
+ fwd => $fwd
+})
+ ->status_is(302)
+ ->header_is('Location' => '/');
+
+$t->get_ok('/')
+ ->status_is(200)
+ ->element_exists('div.notify-error')
+ ->text_is('div.notify-error', 'Invalid character in request')
+ ->element_exists('input[name=handle]:not([value])')
+ ->element_exists_not('div.button.top a')
+ ;
+
+# Login:
+# UTF8 request
+my $username = b('täst')->encode;
+$t->post_ok('/user/login' => form => {
+ handle => $username,
pwd => 'pass',
csrf_token => $csrf
})
@@ -447,6 +469,7 @@
->element_exists_not('div.notify-error')
->element_exists('div.notify-success')
->text_is('div.notify-success', 'Login successful')
+ ->attr_is('a.logout', 'title', "Logout: $username")
->element_exists_not('aside.off')
->element_exists_not('aside.active')
->element_exists('aside.settings')
diff --git a/t/server/mock.pl b/t/server/mock.pl
index 5389156..59a6766 100644
--- a/t/server/mock.pl
+++ b/t/server/mock.pl
@@ -52,7 +52,6 @@
helper expired => sub {
my ($c, $auth, $set) = @_;
-
$auth =~ s/^[^ ]+? //;
if ($set) {
$c->app->log->debug("Set $auth for expiration");
@@ -402,7 +401,7 @@
}
# Check for wrong user name
- elsif ($c->param('username') ne 'test') {
+ elsif ($c->param('username') !~ /^t.st$/) {
return $c->render(json => {
error => [[2004, undef]]
});