Restrict characters in URLs of packages and platforms #6962

crates/packaging/src/https.rs

@@ -45,6 +45,22 @@ const MISLEADING_CHARACTERS_IN_URL: [char; 5] = [
     '\u{29F8}', // U+29F8 == ⧸ Big Solidus
 ];
 
+// Unsafe URL characters that should be encoded can be found here: https://datatracker.ietf.org/doc/html/rfc1738
+const ALLOWED_URL_CHARACTERS: [char; 13] = [
+    '-',
+    '.',
+    '_',
+    ':',
+    '/',
+    '!',
+    '$',
+    '(',
+    ')',
+    '*',
+    '+',
+    ',',
+    '#'
+];
 #[derive(Debug, PartialEq, Eq)]
 pub enum UrlProblem {
     InvalidExtensionSuffix(String),
@@ -53,6 +69,7 @@ pub enum UrlProblem {
     MissingHash,
     MissingHttps,
     MisleadingCharacter,
+    InsecureCharacter((String, usize))
 }
 
 impl<'a> TryFrom<&'a str> for PackageMetadata<'a> {
@@ -73,12 +90,19 @@ impl<'a> PackageMetadata<'a> {
             }
         };
 
-        // Next, check if there are misleading characters in the URL
-        if url
+        if let Some((index, ch)) = url
             .chars()
-            .any(|ch| MISLEADING_CHARACTERS_IN_URL.contains(&ch))
+            .enumerate()
+            .find(|(_, ch)| (!ch.is_alphanumeric() && !ALLOWED_URL_CHARACTERS.contains(ch)) || MISLEADING_CHARACTERS_IN_URL.contains(ch)) 
         {
-            return Err(UrlProblem::MisleadingCharacter);
+            // Check if there is an intentionally misleading url character
+            // Otherwise check if there is a possibly insecure character in the url
+            if MISLEADING_CHARACTERS_IN_URL.contains(&ch) {
+                return Err(UrlProblem::MisleadingCharacter);
+            } else {
+                return Err(UrlProblem::InsecureCharacter((ch.to_string(), index)));
+            }
+
         }
 
         // Next, get the (optional) URL fragment, which must be a .roc filename
@@ -155,6 +179,23 @@ fn url_problem_misleading_characters() {
     }
 }
 
+#[test]
+fn url_problem_insecure_characters() {
+    let expected = [
+        ("https://github.com/roc-l<ang/", Err(UrlProblem::InsecureCharacter(("<".to_string(), 24)))),   
+        ("https://github.com/roc-l>ang/", Err(UrlProblem::InsecureCharacter((">".to_string(), 24)))),
+        ("https://github.com/roc-l`ang/", Err(UrlProblem::InsecureCharacter(("`".to_string(), 24)))),    
+    ];
+
+
+    for (test_url, expected_err) in expected {
+        assert_eq!(
+            PackageMetadata::try_from(test_url),
+            expected_err
+        );
+    }
+}
+
 #[test]
 fn url_problem_invalid_fragment_not_a_roc_file() {
     let expected = Err(UrlProblem::InvalidFragment("filename.sh".to_string()));

crates/reporting/src/report.rs

@@ -1658,6 +1658,39 @@ pub fn to_https_problem_report<'b>(
                 severity: Severity::Fatal,
             }
         }
+        Problem::InvalidUrl(roc_packaging::https::UrlProblem::InsecureCharacter(insecure)) => {
+            let (insecure_char, char_index) = insecure;
+            let doc = alloc.stack([
+                alloc.reflow(r"I tried to download from this URL:"),
+                alloc
+                    .string((&url).to_string())
+                    .annotate(Annotation::Url)
+                    .indent(4),
+                alloc.concat([
+                    alloc.reflow(r"I have found one or more insecure "),
+                    alloc.reflow(r"characters in this URL, the first of which are "),
+                    alloc.string(format!("{} at character {}. ", insecure_char.escape_unicode(), char_index)),
+                ]),
+                alloc.concat([
+                    alloc.reflow(r"If you have a use-case for any of these characters we "),
+                    alloc.reflow(r"would like to hear about it. Reach out on "),
+                    alloc
+                        .string(r"https://github.com/roc-lang/roc/issues/6962".to_string())
+                        .annotate(Annotation::Url),
+                ]),
+                alloc.concat([
+                    alloc.tip(),
+                    alloc.reflow(r"Check that you have the correct URL for this package/platform."),
+                ]),
+            ]);
+
+            Report {
+                filename,
+                doc,
+                title: "INSECURE URL CHARACTERS".to_string(),
+                severity: Severity::Fatal,
+            }
+        }
         Problem::DownloadTooBig(content_len) => {
             let nice_bytes = Byte::from_bytes(content_len.into())
                 .get_appropriate_unit(false)