CVE-2021-29451: Improper Verification of Cryptographic Signature (CWE-347)
value: Portofino is an open source web development framework. Portofino before version 5.2.1 did not properly verify the signature of JSON Web Tokens. This allows forging a valid JWT. The issue will be patched in the upcoming 5.2.1 release.
CWE description: The product does not verify, or incorrectly verifies, the cryptographic signature for data.
CVSS v3 base score: 9.1
Published: 2021-04-16T22:15Z
Repository: https://github.com/ManyDesigns/Portofino
Language: Java
Files changed: CHANGELOG.md, dispatcher/src/main/java/com/manydesigns/portofino/dispatcher/security/jwt/JWTRealm.java, portofino-core/src/main/java/com/manydesigns/portofino/shiro/AbstractPortofinoRealm.java
Commit date: 2021-04-01 18:16:16 +0200
Commit message: Fix security vulnerability

Diff: diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1b6acf06f..befeadada 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,12 +11,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Render internal links in text pages with no target attribute (that the Quill editor adds by default).
 - The welcome page is now a standard text page.
 - Security.groovy can now have user beans injected with @Autowired.
 
 ### Fixed
-- Backwards compatibility: revert `T extends Serializable` in CRUD actions, introduced in v5.2.0. [#428](https://github.com/ManyDesigns/Portofino/issues/428)
+- Important security vulnerability that may have allowed access with forged tokens.
 - Authentication token refresh after expiration. [#430](https://github.com/ManyDesigns/Portofino/issues/430)
+- Backwards compatibility: revert `T extends Serializable` in CRUD actions, introduced in v5.2.0. [#428](https://github.com/ManyDesigns/Portofino/issues/428)
 
 ## [5.2.0] – 2020-11-30
 
 ### Added
 
diff --git a/dispatcher/src/main/java/com/manydesigns/portofino/dispatcher/security/jwt/JWTRealm.java b/dispatcher/src/main/java/com/manydesigns/portofino/dispatcher/security/jwt/JWTRealm.java
index 7fd45e3b2..c51a6d6b2 100644
--- a/dispatcher/src/main/java/com/manydesigns/portofino/dispatcher/security/jwt/JWTRealm.java
+++ b/dispatcher/src/main/java/com/manydesigns/portofino/dispatcher/security/jwt/JWTRealm.java
@@ -1,11 +1,9 @@
 package com.manydesigns.portofino.dispatcher.security.jwt;
 
 import com.manydesigns.portofino.dispatcher.security.RolesPermission;
-import io.jsonwebtoken.Jwt;
-import io.jsonwebtoken.Jwts;
-import io.jsonwebtoken.SignatureAlgorithm;
+import io.jsonwebtoken.*;
 import io.jsonwebtoken.io.Decoders;
 import org.apache.commons.configuration2.Configuration;
 import org.apache.shiro.authc.AuthenticationException;
 import org.apache.shiro.authc.AuthenticationInfo;
 import org.apache.shiro.authc.AuthenticationToken;
@@ -51,22 +49,22 @@ public class JWTRealm extends AuthorizingRealm {
     @Override
     protected AuthenticationInfo doGetAuthenticationInfo(AuthenticationToken token) throws AuthenticationException {
         String secret = getSecret();
         Key key = new SecretKeySpec(Decoders.BASE64.decode(secret), getSignatureAlgorithm().getJcaName());
         
-        Jwt jwt = Jwts.parser().
+        Jws<Claims> jwt = Jwts.parser().
                 setSigningKey(key).
-                parse((String) token.getPrincipal());
+                parseClaimsJws((String) token.getPrincipal());
         Map<String, Serializable> principal = getPrincipal(jwt);
         return new SimpleAuthenticationInfo(principal, ((String) token.getCredentials()).toCharArray(), getName());
     }
 
     protected SignatureAlgorithm getSignatureAlgorithm() {
         return SignatureAlgorithm.HS512;
     }
 
-    protected Map<String, Serializable> getPrincipal(Jwt jwt) {
+    protected Map<String, Serializable> getPrincipal(Jws<Claims> jwt) {
         Map<String, Serializable> principal = new HashMap<>();
         principal.put("jwt", (Serializable) jwt.getBody());
         return principal;
     }
     
diff --git a/portofino-core/src/main/java/com/manydesigns/portofino/shiro/AbstractPortofinoRealm.java b/portofino-core/src/main/java/com/manydesigns/portofino/shiro/AbstractPortofinoRealm.java
index 923cca3d5..0bd777a53 100644
--- a/portofino-core/src/main/java/com/manydesigns/portofino/shiro/AbstractPortofinoRealm.java
+++ b/portofino-core/src/main/java/com/manydesigns/portofino/shiro/AbstractPortofinoRealm.java
@@ -22,14 +22,11 @@ package com.manydesigns.portofino.shiro;
 
 import com.manydesigns.elements.reflection.ClassAccessor;
 import com.manydesigns.elements.reflection.JavaClassAccessor;
 import com.manydesigns.portofino.code.CodeBase;
 import com.manydesigns.portofino.security.SecurityLogic;
-import io.jsonwebtoken.Jwt;
-import io.jsonwebtoken.JwtException;
-import io.jsonwebtoken.Jwts;
-import io.jsonwebtoken.SignatureAlgorithm;
+import io.jsonwebtoken.*;
 import io.jsonwebtoken.io.Decoders;
 import org.apache.commons.configuration2.Configuration;
 import org.apache.shiro.authc.AuthenticationException;
 import org.apache.shiro.authc.AuthenticationInfo;
 import org.apache.shiro.authc.AuthenticationToken;
@@ -98,23 +95,23 @@ public abstract class AbstractPortofinoRealm extends AuthorizingRealm implements
         return token instanceof JSONWebToken || super.supports(token);
     }
 
     public AuthenticationInfo loadAuthenticationInfo(JSONWebToken token) {
         Key key = getJWTKey();
-        Jwt jwt;
+        Jws<Claims> jwt;
         try {
-            jwt = Jwts.parser().setSigningKey(key).parse(token.getPrincipal());
+            jwt = Jwts.parser().setSigningKey(key).parseClaimsJws(token.getPrincipal());
         } catch (JwtException e) {
             throw new AuthenticationException(e);
         }
         String credentials = legacyHashing ? token.getCredentials() : encryptPassword(token.getCredentials());
         Object principal = extractPrincipalFromWebToken(jwt);
         return new SimpleAuthenticationInfo(principal, credentials, getName());
     }
 
-    protected Object extractPrincipalFromWebToken(Jwt jwt) {
-        Map body = (Map) jwt.getBody();
+    protected Object extractPrincipalFromWebToken(Jws<Claims> jwt) {
+        Map<String, Object> body = jwt.getBody();
         String base64Principal = (String) body.get("serialized-principal");
         byte[] serializedPrincipal = Base64.decode(base64Principal);
         Object principal;
         ClassLoader loader = Thread.currentThread().getContextClassLoader();
         try {

Vulnerable code: - Backwards compatibility: revert `T extends Serializable` in CRUD actions, introduced in v5.2.0. [#428](https://github.com/ManyDesigns/Portofino/issues/428)
import io.jsonwebtoken.Jwt;
import io.jsonwebtoken.Jwts;
import io.jsonwebtoken.SignatureAlgorithm;
        Jwt jwt = Jwts.parser().
                parse((String) token.getPrincipal());
    protected Map<String, Serializable> getPrincipal(Jwt jwt) {
import io.jsonwebtoken.Jwt;
import io.jsonwebtoken.JwtException;
import io.jsonwebtoken.Jwts;
import io.jsonwebtoken.SignatureAlgorithm;
        Jwt jwt;
            jwt = Jwts.parser().setSigningKey(key).parse(token.getPrincipal());
    protected Object extractPrincipalFromWebToken(Jwt jwt) {
        Map body = (Map) jwt.getBody();
Fixed code: - Important security vulnerability that may have allowed access with forged tokens.
- Backwards compatibility: revert `T extends Serializable` in CRUD actions, introduced in v5.2.0. [#428](https://github.com/ManyDesigns/Portofino/issues/428)
import io.jsonwebtoken.*;
        Jws<Claims> jwt = Jwts.parser().
                parseClaimsJws((String) token.getPrincipal());
    protected Map<String, Serializable> getPrincipal(Jws<Claims> jwt) {
import io.jsonwebtoken.*;
        Jws<Claims> jwt;
            jwt = Jwts.parser().setSigningKey(key).parseClaimsJws(token.getPrincipal());
    protected Object extractPrincipalFromWebToken(Jws<Claims> jwt) {
        Map<String, Object> body = jwt.getBody();