utils: Make sanitise_path() sanitise harder

Message ID 20180912045530.16861-1-andrew.donnellan@au1.ibm.com
State Accepted
Headers show
Series
  • utils: Make sanitise_path() sanitise harder
Related show

Commit Message

Andrew Donnellan Sept. 12, 2018, 4:55 a.m.
The current list of characters to sanitise in sanitise_path() is
incomplete, as we learned when we saw a build fail because someone had
included < in their patch summary.

Rather than maintain a list of dangerous characters which might be
problematic, let's just attack this with a giant hammer and filter
everything that's not alphanumeric as defined by Rust.

Closes: #48 ("Normalise branch names to reduce shell injection vulnerabilities")
Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
---
 src/utils.rs | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

Comments

Russell Currey Nov. 9, 2018, 4:56 a.m. | #1
On Wed, 2018-09-12 at 14:55 +1000, Andrew Donnellan wrote:
> The current list of characters to sanitise in sanitise_path() is
> incomplete, as we learned when we saw a build fail because someone
> had
> included < in their patch summary.
> 
> Rather than maintain a list of dangerous characters which might be
> problematic, let's just attack this with a giant hammer and filter
> everything that's not alphanumeric as defined by Rust.
> 
> Closes: #48 ("Normalise branch names to reduce shell injection
> vulnerabilities")
> Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

Sorry I let this slip.  Should've been doing this from the start, I am
dumb.

Merged to master as 1da364f3d620ac8014c3c13cac2f7bf6207c783c

Patch

diff --git a/src/utils.rs b/src/utils.rs
index 5f791afd59eb..ec98bb49bbb9 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -14,18 +14,5 @@ 
 //
 
 pub fn sanitise_path(path: &str) -> String {
-    path.replace("/", "_")
-        .replace("\\", "_")
-        .replace(".", "_")
-        .replace("~", "_")
-        .replace(" ", "_")
-        .replace(":", "")
-        .replace("[", "_")
-        .replace("]", "_")
-        .replace("'", "")
-        .replace("\"", "")
-        .replace("(", "_")
-        .replace(")", "_")
-        .replace("*", "_")
-        .replace("?", "_")
+    path.replace(|c: char| !c.is_alphanumeric(), "_")
 }