fix(nix-compat/narinfo/signature): validate name field r/7075

We should restrict this to alphanumeric mostly, and we definitely don't
want newlines.

Not entirely sure about the exact additionally allowed characters
outside of alphanumeric, but this can always be extended further.

Change-Id: I1357e79e553f2df2fa97792889f63f0f35d50ed5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10147
Reviewed-by: edef <edef@edef.eu>
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>

1 files changed, 15 insertions, 2 deletions

diff --git a/tvix/nix-compat/src/narinfo/signature.rs b/tvix/nix-compat/src/narinfo/signature.rs
index 53e75c5873..beace4aa81 100644
--- a/tvix/nix-compat/src/narinfo/signature.rs
+++ b/tvix/nix-compat/src/narinfo/signature.rs
@@ -5,7 +5,6 @@ use ed25519_dalek::SIGNATURE_LENGTH;
 
 #[derive(Debug)]
 pub struct Signature<'a> {
-    /// TODO(edef): be stricter with signature names here, they especially shouldn't have newlines!
     name: &'a str,
     bytes: [u8; SIGNATURE_LENGTH],
 }
@@ -20,6 +19,14 @@ impl<'a> Signature<'a> {
             .split_once(':')
             .ok_or(SignatureError::MissingSeparator)?;
 
+        if name.is_empty()
+            || !name
+                .chars()
+                .all(|c| char::is_alphanumeric(c) || c == '-' || c == '.')
+        {
+            return Err(SignatureError::InvalidName(name.to_string()));
+        }
+
         if bytes64.len() != BASE64.encode_len(SIGNATURE_LENGTH) {
             return Err(SignatureError::InvalidSignatureLen(bytes64.len()));
         }
@@ -54,6 +61,8 @@ impl<'a> Signature<'a> {
 
 #[derive(Debug, thiserror::Error)]
 pub enum SignatureError {
+    #[error("Invalid name: {0}")]
+    InvalidName(String),
     #[error("Missing separator")]
     MissingSeparator,
     #[error("Invalid signature len: (expected {} b64-encoded, got {}", BASE64.encode_len(SIGNATURE_LENGTH), .0)]
@@ -114,7 +123,11 @@ mod test {
         assert_eq!(expect_valid, sig.verify(fp.as_bytes(), verifying_key));
     }
 
-    #[test_case("cache.nixos.org-1:o1DTsjCz0PofLJ216P2RBuSulI8BAb6zHxWE4N+tzlcELk5Uk/GO2SCxWTRN5wJutLZZ+cHTMdWqOHF8"; "wrong_length")]
+    #[test_case("cache.nixos.org-1:o1DTsjCz0PofLJ216P2RBuSulI8BAb6zHxWE4N+tzlcELk5Uk/GO2SCxWTRN5wJutLZZ+cHTMdWqOHF8"; "wrong length")]
+    #[test_case("test\n:u01BybwQhyI5H1bW1EIWXssMDhDDIvXOG5uh8Qzgdyjz6U1qg6DHhMAvXZOUStIj6X5t4/ufFgR8i3fjf0bMAw=="; "wrong name newline")]
+    #[test_case("test :u01BybwQhyI5H1bW1EIWXssMDhDDIvXOG5uh8Qzgdyjz6U1qg6DHhMAvXZOUStIj6X5t4/ufFgR8i3fjf0bMAw=="; "wrong name space")]
+    #[test_case(":u01BybwQhyI5H1bW1EIWXssMDhDDIvXOG5uh8Qzgdyjz6U1qg6DHhMAvXZOUStIj6X5t4/ufFgR8i3fjf0bMAw=="; "empty name")]
+    #[test_case("u01BybwQhyI5H1bW1EIWXssMDhDDIvXOG5uh8Qzgdyjz6U1qg6DHhMAvXZOUStIj6X5t4/ufFgR8i3fjf0bMAw=="; "b64 only")]
     fn parse_fail(input: &'static str) {
         Signature::parse(input).expect_err("must fail");
     }