Catch JSON errors (#1386)

lemonldap-ng-portal/lib/Lemonldap/NG/Portal/2F/Register/TOTP.pm

@@ -85,7 +85,7 @@ sub run {
             $token->{_totp2fSecret}, $code
         );
         if ( $r == -1 ) {
-            return $self->p->sendError( 'serverError', 500 );
+            return $self->p->sendError( $req, 'serverError', 500 );
         }
 
         # Invalid try is returned with a 200 code. Javascript will read error
@@ -99,14 +99,22 @@ sub run {
 
         # Now code is verified, let's store the master key in persistent data
 
-        my $secret     = '';
-        my $_2fDevices = eval {
-            $self->logger->debug("Looking for 2F Devices ...");
+        my $secret = '';
 
-            # Reading existing 2FDevices
-            from_json( $req->userData->{_2fDevices}, { allow_nonref => 1 } );
-        };
-        unless ($_2fDevices) {
+        # Reading existing 2FDevices
+        $self->logger->debug("Looking for 2F Devices ...");
+        my $_2fDevices;
+        if ( $req->userData->{_2fDevices} ) {
+            $_2fDevices = eval {
+                from_json( $req->userData->{_2fDevices},
+                    { allow_nonref => 1 } );
+            };
+            if ($@) {
+                $self->logger->error("Corrupted session (_2fDevices): $@");
+                return $self->p->sendError( $req, "Corrupted session", 500 );
+            }
+        }
+        else {
             $self->logger->debug("No 2F Device found");
             $_2fDevices = [];
         }
@@ -172,12 +180,19 @@ sub run {
         my $nk     = 0;
         my $secret = '';
 
-        my $_2fDevices = eval {
-            $self->logger->debug("Loading 2F Devices ...");
-
-            # Read existing 2FDevices
-            from_json( $req->userData->{_2fDevices}, { allow_nonref => 1 } );
-        };
+        # Read existing 2FDevices
+        $self->logger->debug("Loading 2F Devices ...");
+        my $_2fDevices = [];
+        if ( $req->userData->{_2fDevices} ) {
+            $_2fDevices = eval {
+                from_json( $req->userData->{_2fDevices},
+                    { allow_nonref => 1 } );
+            };
+            if ($@) {
+                $self->logger->error("Corrupted session (_2fDevices): $@");
+                return $self->p->sendError( $req, "Corrupted session", 500 );
+            }
+        }
 
         # Loading TOTP secret
         my @totp2f = grep { $_->{type} eq "TOTP" } @$_2fDevices;
@@ -250,13 +265,21 @@ sub run {
 
     # Delete TOTP
     if ( $action eq 'delete' ) {
-        my $epoch      = $req->param('epoch');
-        my $_2fDevices = eval {
-            $self->logger->debug("Loading 2F Devices ...");
-
-            # Read existing 2FDevices
-            from_json( $req->userData->{_2fDevices}, { allow_nonref => 1 } );
-        };
+        my $epoch = $req->param('epoch');
+
+        # Read existing 2FDevices
+        $self->logger->debug("Loading 2F Devices ...");
+        my $_2fDevices = [];
+        if ( $req->userData->{_2fDevices} ) {
+            $_2fDevices = eval {
+                from_json( $req->userData->{_2fDevices},
+                    { allow_nonref => 1 } );
+            };
+            if ($@) {
+                $self->logger->error("Corrupted session (_2fDevices): $@");
+                return $self->p->sendError( $req, "Corrupted session", 500 );
+            }
+        }
 
         # Delete TOTP 2F device
         my @keep = ();

lemonldap-ng-portal/lib/Lemonldap/NG/Portal/2F/TOTP.pm

@@ -65,27 +65,31 @@ sub verify {
         return PE_FORMEMPTY;
     }
 
-    my $secret     = '';
-    my $_2fDevices = eval {
+    my $secret = '';
+    my $_2fDevices;
+    if ( $session->{_2fDevices} ) {
         $self->logger->debug("Loading 2F Devices ...");
 
         # Read existing 2FDevices
-        from_json( $session->{_2fDevices}, { allow_nonref => 1 } );
-    };
-
-    unless ($_2fDevices) {
-        $self->logger->debug("No 2F Device found");
-
-        # Set default value
-        @$_2fDevices = [];
+        $_2fDevices =
+          eval { from_json( $session->{_2fDevices}, { allow_nonref => 1 } ); };
+        if ($@) {
+            $self->logger->error("Bad encoding in _2fDevices: $@");
+            return PE_ERROR;
+        }
+        $self->logger->debug("2F Device(s) found");
+        foreach (@$_2fDevices) {
+            $self->logger->debug("Reading TOTP secret if exists ...");
+            if ( $_->{type} eq 'TOTP' ) {
+                $secret = $_->{_secret};
+                last;
+            }
+        }
     }
 
-    foreach (@$_2fDevices) {
-        $self->logger->debug("Reading TOTP secret if exists ...");
-        if ( $_->{type} eq 'TOTP' ) {
-            $secret = $_->{_secret};
-            last;
-        }
+    unless ($secret) {
+        $self->logger->debug("No TOTP secret found");
+        return PE_BADCREDENTIALS;
     }
 
     my $r = $self->verifyCode(

lemonldap-ng-portal/t/71-2F-UTOTP-TOTP-only.t

@@ -8,7 +8,7 @@ my $maintests = 16;
 SKIP: {
     eval { require Convert::Base32; require Crypt::U2F::Server::Simple; };
     if ($@) {
-        skip 'Convert::Base32 is missing', $maintests;
+        skip 'Missing libraries', $maintests;
     }
     require Lemonldap::NG::Common::TOTP;