From 710ea2346f535496542c37f49be2b6e8f0378926 Mon Sep 17 00:00:00 2001
From: Noel Cower <ncower@nil.dev>
Date: Fri, 23 Feb 2024 08:23:36 -0800
Subject: [PATCH] fix: do not parse base62 strings of unexpected length

Reject parsing base62 strings that would result in unexpectedly
long payloads and potentially large allocations. Without this, it
is possible to use a base62 string to pass in extra bytes of data
that are unused (but do consume memory for a time). I don't know
if this is something people rely on, but it seems unlikely that
anyone would find this desirable. This might represent a possible
way to OOM any code accepting KSUIDs that didn't limit incoming
payload sizes already, but there are probably better options for that.

Add a test for both long and short base62 strings to try to skip
allocating anything for strings of the wrong length by decoding
them. Previously, length checks only happened after decoding and
would only take the tail end of the decoded base62 string.
---
 src/lib.rs           |  7 +++++++
 tests/integration.rs | 12 ++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/src/lib.rs b/src/lib.rs
index 635ad9b..908d3e8 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -76,6 +76,7 @@ pub const KSUID_EPOCH: i64 = 1_400_000_000;
 const BASE_62_CHARS: &[u8; 62] = b"0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";
 
 const TOTAL_BYTES: usize = 20;
+const TOTAL_BYTES_BASE62: usize = 27;
 
 #[derive(Debug)]
 pub struct Error(String);
@@ -237,6 +238,12 @@ pub trait KsuidLike {
     ///
     /// ```
     fn from_base62(s: &str) -> Result<Self::Type, Error> {
+        if s.len() != TOTAL_BYTES_BASE62 {
+            return Err(Error(format!(
+                "Got base62 ksuid of unexpected length {}",
+                s.len()
+            )));
+        }
         if let Some(loaded) = base_encode::from_str(s, 62, BASE_62_CHARS) {
             // Get the last TOTAL_BYTES
             let loaded = if loaded.len() > TOTAL_BYTES {
diff --git a/tests/integration.rs b/tests/integration.rs
index a109a9a..9cfabb7 100644
--- a/tests/integration.rs
+++ b/tests/integration.rs
@@ -203,3 +203,15 @@ fn test_deserialize_from_base62() {
     assert_eq!(ksuid_obj.id.to_string(), b62);
     assert_eq!(ksuidms_obj.id.to_string(), b62);
 }
+
+#[test]
+fn test_deserialize_bad_base62_length() {
+    let short_b62 = "ZBpBUvZwXKQmoEYga2";
+    let long_b62 = "1srOrx2ZWZBpBUvZwXKQmoEYga21srOrx2ZWZBpBUvZwXKQmoEYga2";
+
+    let result = Ksuid::from_base62(short_b62);
+    assert!(result.is_err(), "Short base62 strings should fail to parse");
+
+    let result = Ksuid::from_base62(long_b62);
+    assert!(result.is_err(), "Long base62 strings should fail to parse");
+}