From 72fb766ad446120a7a35aa795a3b1bd3addbbf52 Mon Sep 17 00:00:00 2001
From: Pol Henarejos <pol.henarejos@cttc.es>
Date: Fri, 14 Aug 2026 14:22:10 +0200
Subject: [PATCH] Reject out-of-range new PINs with SW_CONDITIONS_NOT_SATISFIED

Signed-off-by: Pol Henarejos <pol.henarejos@cttc.es>
---
 src/openpgp/cmd_change_pin.c  |  8 ++++++++
 src/openpgp/cmd_put_data.c    |  3 +++
 src/openpgp/cmd_reset_retry.c | 10 ++++++++--
 src/openpgp/openpgp.c         |  8 ++++++++
 src/openpgp/openpgp.h         |  1 +
 5 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/src/openpgp/cmd_change_pin.c b/src/openpgp/cmd_change_pin.c
index 4426f59..1ef5815 100644
--- a/src/openpgp/cmd_change_pin.c
+++ b/src/openpgp/cmd_change_pin.c
@@ -56,6 +56,9 @@ int cmd_change_pin(void) {
         return SW_REFERENCE_NOT_FOUND();
     }
     uint8_t pin_len = file_get_data(pw)[0];
+    if (apdu.nc < pin_len) {
+        return SW_WRONG_LENGTH();
+    }
     uint16_t r = 0;
     r = check_pin(pw, apdu.data, pin_len);
     if (r != 0x9000) {
@@ -67,6 +70,7 @@ int cmd_change_pin(void) {
 
     const uint8_t *new_pin = apdu.data + pin_len;
     size_t new_pin_len = apdu.nc - pin_len;
+    bool allow_empty_new_pin = false;
 #ifdef ENABLE_ADMINLESS_MODE
     /* Empty PW3 is Gnuk's "PW3 not configured" transition. Keep its
      * current verifier so KDF-backed host flows can still verify it. */
@@ -75,10 +79,14 @@ int cmd_change_pin(void) {
         new_pin = apdu.data;
         new_pin_len = pin_len;
     }
+    allow_empty_new_pin = clear_pw3;
     bool sync_adminless_pw3 = P2(apdu) == 0x81 && (openpgp_adminless_is_active() || (openpgp_adminless_is_pending() && new_pin_len >= 8));
     bool disable_pending_adminless = P2(apdu) == 0x81 && openpgp_adminless_is_pending() && new_pin_len < 8;
     bool enable_adminless = P2(apdu) == 0x83 && !clear_pw3 && pw3_matches_nonfactory_pw1(new_pin, new_pin_len);
 #endif
+    if (!allow_empty_new_pin && (r = check_pin_len(fid, new_pin_len)) != 0x9000) {
+        return r;
+    }
 
     uint8_t dhash[34];
     dhash[0] = new_pin_len;
diff --git a/src/openpgp/cmd_put_data.c b/src/openpgp/cmd_put_data.c
index 62825f2..0be1297 100644
--- a/src/openpgp/cmd_put_data.c
+++ b/src/openpgp/cmd_put_data.c
@@ -89,6 +89,9 @@ int cmd_put_data(void) {
                 r = file_put_data(ef, CONST_BYTE_ARRAY(pw_status, sizeof(pw_status)));
             }
             else if (fid == EF_RC) {
+                if ((r = check_pin_len(EF_RC, apdu.nc)) != 0x9000) {
+                    return r;
+                }
                 has_rc = false;
                 if ((r = load_dek()) != PICOKEYS_OK) {
                     return SW_EXEC_ERROR();
diff --git a/src/openpgp/cmd_reset_retry.c b/src/openpgp/cmd_reset_retry.c
index 1be59c5..bb23c08 100644
--- a/src/openpgp/cmd_reset_retry.c
+++ b/src/openpgp/cmd_reset_retry.c
@@ -29,6 +29,7 @@ int cmd_reset_retry(void) {
         bool sync_adminless_pw3 = openpgp_adminless_is_active();
 #endif
         has_pw1 = false;
+        uint16_t r = 0;
         if (!(pw = file_search_by_fid(EF_PW1, NULL, SPECIFY_EF))) {
             return SW_REFERENCE_NOT_FOUND();
         }
@@ -44,11 +45,14 @@ int cmd_reset_retry(void) {
             if (apdu.nc <= pin_len) {
                 return SW_WRONG_LENGTH();
             }
-            uint16_t r = check_pin(rc, apdu.data, pin_len);
+            r = check_pin(rc, apdu.data, pin_len);
             if (r != 0x9000) {
                 return r;
             }
             newpin_len = apdu.nc - pin_len;
+            if ((r = check_pin_len(EF_PW1, newpin_len)) != 0x9000) {
+                return r;
+            }
             has_rc = true;
             pin_derive_session(CONST_BYTE_ARRAY(apdu.data, pin_len), session_rc);
             has_pw1 = has_pw3 = false;
@@ -59,8 +63,10 @@ int cmd_reset_retry(void) {
                 return SW_CONDITIONS_NOT_SATISFIED();
             }
             newpin_len = apdu.nc;
+            if ((r = check_pin_len(EF_PW1, newpin_len)) != 0x9000) {
+                return r;
+            }
         }
-        int r = 0;
         if ((r = load_dek()) != PICOKEYS_OK) {
             return SW_EXEC_ERROR();
         }
diff --git a/src/openpgp/openpgp.c b/src/openpgp/openpgp.c
index 29536ba..0d68e79 100644
--- a/src/openpgp/openpgp.c
+++ b/src/openpgp/openpgp.c
@@ -196,6 +196,14 @@ static bool pin_record_matches_value(const file_t *pin, const uint8_t *value, si
     return false;
 }
 
+int check_pin_len(uint16_t fid, size_t len) {
+    size_t min_len = fid == EF_PW1 ? 6u : 8u;
+    if (len < min_len || len > 127u) {
+        return SW_CONDITIONS_NOT_SATISFIED();
+    }
+    return SW_OK();
+}
+
 static bool reset_code_is_public_default(const file_t *rc) {
     static const uint8_t default_reset_code[] = "12345678";
 
diff --git a/src/openpgp/openpgp.h b/src/openpgp/openpgp.h
index b3c4b87..f1018a2 100644
--- a/src/openpgp/openpgp.h
+++ b/src/openpgp/openpgp.h
@@ -64,6 +64,7 @@ extern int load_dek(void);
 extern void release_dek(void);
 extern bool piv_key_operation_authorized(uint16_t operation, bool internal_firmware);
 extern int check_pin(const file_t *pin, const uint8_t *data, size_t len);
+extern int check_pin_len(uint16_t fid, size_t len);
 extern int openpgp_reset_code_deactivate(void);
 #ifdef ENABLE_ADMINLESS_MODE
 extern bool openpgp_adminless_is_pending(void);