update isValid

Yubico · May 10, 2024 · 9ef758b · 9ef758b
1 parent 2f583c3
commit 9ef758b
Show file tree

Hide file tree

Showing 2 changed files with 95 additions and 27 deletions.
diff --git a/oath/src/main/java/com/yubico/yubikit/oath/Base32.java b/oath/src/main/java/com/yubico/yubikit/oath/Base32.java
@@ -26,7 +26,7 @@
  */
 @SuppressWarnings("SpellCheckingInspection")
 public class Base32 {
-    private static final char[] ALPHABET = "ABCDEFGHIJKLMNOPQRSTUVWXYZ234567=".toCharArray();
+    private static final char[] ALPHABET = "ABCDEFGHIJKLMNOPQRSTUVWXYZ234567".toCharArray();
 
     private static final char PADDING = '=';
 
@@ -64,7 +64,26 @@ public static String encode(byte[] data) {
     }
 
     public static boolean isValid(String encoded) {
-        return encoded.matches("[A-Z2-7=]*");
+        if (!encoded.matches("^$|^[A-Z2-7]{2,}(=*)$")) {
+            return false;
+        }
+
+        int finalUnitLength = encoded.length() % 8;
+        int paddingIndex = encoded.indexOf("=");
+        if (paddingIndex == -1) {
+            // no padding; input is valid only if a correct padding can be appended
+            return finalUnitLength == 0 || finalUnitLength == 7 || finalUnitLength == 5 ||
+                    finalUnitLength == 4 || finalUnitLength == 2;
+        }
+
+        if (finalUnitLength != 0) {
+            // wrong input length
+            return false;
+        }
+
+        int paddingLength = encoded.length() - paddingIndex;
+        // padding can only be 1, 3, 4 or 6 characters long, otherwise this is not a valid input
+        return paddingLength == 1 || paddingLength == 3 || paddingLength == 4 || paddingLength == 6;
     }
 
     public static byte[] decode(String encoded) {
@@ -106,17 +125,6 @@ public static byte[] decode(String encoded) {
             byte v6 = getValue(c6);
             byte v7 = getValue(c7);
 
-            // validate padding characters
-            if (c0 == PADDING || c1 == PADDING) {
-                throw new IllegalArgumentException("Invalid base32 padding");
-            }
-
-            if ((c2 == PADDING && c3 != PADDING) ||
-                    (c3 == PADDING && c4 != PADDING) || (c4 == PADDING && c5 != PADDING) ||
-                    (c5 == PADDING && c6 != PADDING) || (c6 == PADDING && c7 != PADDING)) {
-                throw new IllegalArgumentException("Non-base32 digit found");
-            }
-
             // build result until padding is found
             bb.put((byte) (v0 << 3 | v1 >> 2));
             if (c2 == PADDING) break;

diff --git a/oath/src/test/java/com/yubico/yubikit/oath/Base32Test.java b/oath/src/test/java/com/yubico/yubikit/oath/Base32Test.java
@@ -32,15 +32,37 @@ public class Base32Test {
     @Test
     public void testValidInput() {
         assertTrue(Base32.isValid(""));
-        assertTrue(Base32.isValid("ABCDEFGHIJKLMNOPQRSTUVWXYZ234567="));
+        assertTrue(Base32.isValid("ABCDEFGHIJKLMNOPQRSTUVWXYZ234567"));
+        assertTrue(Base32.isValid("AA======"));
+        assertTrue(Base32.isValid("MZXQ===="));
+        assertTrue(Base32.isValid("AA"));
+        assertTrue(Base32.isValid("AAAA"));
+        assertTrue(Base32.isValid("AAAAA"));
+        assertTrue(Base32.isValid("AAAAAAA"));
+
     }
 
     @Test
     public void testInvalidInput() {
         assertFalse(Base32.isValid("0189"));
         assertFalse(Base32.isValid(";.*"));
         assertFalse(Base32.isValid("😀"));
-        assertFalse(Base32.isValid("abcdefghijklmnopqrstuvwxyz234567="));
+        assertFalse(Base32.isValid("abcdefghijklmnopqrstuvwxyz234567"));
+        assertFalse(Base32.isValid("AA="));
+        assertFalse(Base32.isValid("AA=="));
+        assertFalse(Base32.isValid("AA==="));
+        assertFalse(Base32.isValid("AA===="));
+        assertFalse(Base32.isValid("AA====="));
+        assertFalse(Base32.isValid("AA======="));
+        assertFalse(Base32.isValid("A"));
+        assertFalse(Base32.isValid("AAA"));
+        assertFalse(Base32.isValid("AAAAAA"));
+        assertFalse(Base32.isValid("="));
+        assertFalse(Base32.isValid("=="));
+        assertFalse(Base32.isValid("==="));
+        assertFalse(Base32.isValid("AAAAAAA=A"));
+        assertFalse(Base32.isValid("MZ=XW6YTB"));
+        assertFalse(Base32.isValid("MZXQ=="));
     }
 
     @Test
@@ -60,41 +82,79 @@ public void testEncode() {
 
     @Test
     public void testDecode() {
-        assertArrayEquals("".getBytes(), Base32.decode(""));
-        assertArrayEquals("f".getBytes(), Base32.decode("MY"));
         assertArrayEquals("f".getBytes(), Base32.decode("MY======"));
-        assertArrayEquals("fo".getBytes(), Base32.decode("MZXQ"));
         assertArrayEquals("fo".getBytes(), Base32.decode("MZXQ===="));
-        assertArrayEquals("foo".getBytes(), Base32.decode("MZXW6"));
         assertArrayEquals("foo".getBytes(), Base32.decode("MZXW6==="));
-        assertArrayEquals("foob".getBytes(), Base32.decode("MZXW6YQ"));
         assertArrayEquals("foob".getBytes(), Base32.decode("MZXW6YQ="));
         assertArrayEquals("fooba".getBytes(), Base32.decode("MZXW6YTB"));
-        assertArrayEquals("foobar".getBytes(), Base32.decode("MZXW6YTBOI"));
         assertArrayEquals("foobar".getBytes(), Base32.decode("MZXW6YTBOI======"));
-        assertArrayEquals("yubikit-sdk 2.X.0   !".getBytes(), Base32.decode("PF2WE2LLNF2C243ENMQDELSYFYYCAIBAEE"));
         assertArrayEquals("yubikit-sdk 2.X.0   !".getBytes(), Base32.decode("PF2WE2LLNF2C243ENMQDELSYFYYCAIBAEE======"));
         assertArrayEquals("The quick brown fox jumps over the lazy dog.".getBytes(), Base32.decode("KRUGKIDROVUWG2ZAMJZG653OEBTG66BANJ2W24DTEBXXMZLSEB2GQZJANRQXU6JAMRXWOLQ="));
-        assertArrayEquals(Codec.fromHex("b6481c8be8f22179b58a"), Base32.decode("WZEBZC7I6IQXTNMK"));
         assertArrayEquals(Codec.fromHex("69b6481c8baba2b60e8f22179b58cd56"), Base32.decode("NG3EQHELVORLMDUPEILZWWGNKY======"));
     }
 
-    @Test //(expected = IllegalArgumentException.class)
+    @Test
+    public void testDecodeWithoutPadding() {
+        assertArrayEquals("".getBytes(), Base32.decode(""));
+        assertArrayEquals("f".getBytes(), Base32.decode("MY"));
+        assertArrayEquals("fo".getBytes(), Base32.decode("MZXQ"));
+        assertArrayEquals("foo".getBytes(), Base32.decode("MZXW6"));
+        assertArrayEquals("foob".getBytes(), Base32.decode("MZXW6YQ"));
+        assertArrayEquals("fooba".getBytes(), Base32.decode("MZXW6YTB"));
+        assertArrayEquals("foobar".getBytes(), Base32.decode("MZXW6YTBOI"));
+        assertArrayEquals("yubikit-sdk 2.X.0   !".getBytes(), Base32.decode("PF2WE2LLNF2C243ENMQDELSYFYYCAIBAEE"));
+        assertArrayEquals("The quick brown fox jumps over the lazy dog.".getBytes(), Base32.decode("KRUGKIDROVUWG2ZAMJZG653OEBTG66BANJ2W24DTEBXXMZLSEB2GQZJANRQXU6JAMRXWOLQ"));
+        assertArrayEquals(Codec.fromHex("b6481c8be8f22179b58a"), Base32.decode("WZEBZC7I6IQXTNMK"));
+    }
+
+    @Test
     public void testDecodeThrows() {
+        byte[] invalid = new byte[]{0};
+
+        try {
+            assertArrayEquals(invalid, Base32.decode("invalidinput"));
+            fail();
+        } catch (IllegalArgumentException ignored) {
+        }
+
+        try {
+            assertArrayEquals(invalid, Base32.decode("M="));
+            fail();
+        } catch (IllegalArgumentException ignored) {
+        }
+
+        try {
+            assertArrayEquals(invalid, Base32.decode("A=A"));
+            fail();
+        } catch (IllegalArgumentException ignored) {
+        }
+
+        try {
+            assertArrayEquals(invalid, Base32.decode("="));
+            fail();
+        } catch (IllegalArgumentException ignored) {
+        }
+
+        try {
+            assertArrayEquals(invalid, Base32.decode("MZXQ=="));
+            fail();
+        } catch (IllegalArgumentException ignored) {
+        }
+
         try {
-            assertArrayEquals("invalid".getBytes(), Base32.decode("invalidinput"));
+            assertArrayEquals(invalid, Base32.decode("A"));
             fail();
         } catch (IllegalArgumentException ignored) {
         }
 
         try {
-            assertArrayEquals("".getBytes(), Base32.decode("M="));
+            assertArrayEquals(invalid, Base32.decode("AAA"));
             fail();
         } catch (IllegalArgumentException ignored) {
         }
 
         try {
-            assertArrayEquals("".getBytes(), Base32.decode("A=A"));
+            assertArrayEquals(invalid, Base32.decode("AAAAAA"));
             fail();
         } catch (IllegalArgumentException ignored) {
         }