From: Junio C Hamano <junkio@cox.net>
Date: Sun, 17 Apr 2005 04:29:45 +0000 (-0700)
Subject: [PATCH] show-diff shell safety
X-Git-Tag: v0.99~860
X-Git-Url: https://git.verplant.org/?a=commitdiff_plain;h=9bd94cafdf104e613d7609401e60711cd08e209c;p=git.git

[PATCH] show-diff shell safety

The command line for running "diff" command is built without
taking shell metacharacters into account.  A malicious dircache
entry "foo 2>bar" (yes, a filename with space) would result in
creating a file called "bar" with the error message "diff: foo:
No such file or directory" in it.

This is not just a user screwing over himself.  Such a dircache
can be created as a result of a merge with tree from others.

Here is a fix.

Signed-off-by: Junio C Hamano <junkio@cox.net>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---

diff --git a/show-diff.c b/show-diff.c
index 4221d3ae..f2d352f4 100644
--- a/show-diff.c
+++ b/show-diff.c
@@ -4,14 +4,70 @@
  * Copyright (C) Linus Torvalds, 2005
  */
 #include "cache.h"
+#include <ctype.h>
 
-static void show_differences(char *name,
-	void *old_contents, unsigned long long old_size)
+static char *diff_cmd = "diff -L '%s' -u -N  - '%s'";
+
+/* Help to copy the thing properly quoted for the shell safety.
+ * any single quote is replaced with '\'', and the caller is
+ * expected to enclose the result within a single quote pair.
+ *
+ * E.g.
+ *  original     sq_expand     result
+ *  name     ==> name      ==> 'name'
+ *  a b      ==> a b       ==> 'a b'
+ *  a'b      ==> a'\''b    ==> 'a'\''b'
+ *
+ * NOTE! The returned memory belongs to this function so
+ * do not free it.
+ */
+static char *sq_expand(char *src)
+{
+	static char *buf = NULL;
+	static int buf_size = -1;
+	int cnt, c;
+	char *cp;
+
+	/* count single quote characters */ 
+	for (cnt = 0, cp = src; *cp; cnt++, cp++)
+		if (*cp == '\'')
+			cnt += 3;
+
+	if (buf_size < cnt) {
+		free(buf);
+		buf_size = cnt;
+		buf = malloc(cnt);
+	}
+
+	cp = buf;
+	while ((c = *src++)) {
+		if (c != '\'')
+			*cp++ = c;
+		else {
+			cp = strcpy(cp, "'\\''");
+			cp += 4;
+		}
+	}
+	*cp = 0;
+	return buf;
+}
+
+static void show_differences(char *name, void *old_contents,
+			     unsigned long long old_size)
 {
-	static char cmd[1000];
 	FILE *f;
+	static char *cmd = NULL;
+	static int cmd_size = -1;
 
-	snprintf(cmd, sizeof(cmd), "diff -L %s -u -N  - %s", name, name);
+	char *name_sq = sq_expand(name);
+	int cmd_required_length = strlen(name_sq) * 2 + strlen(diff_cmd);
+
+	if (cmd_size < cmd_required_length) {
+		free(cmd);
+		cmd_size = cmd_required_length;
+		cmd = malloc(cmd_required_length);
+	}
+	snprintf(cmd, cmd_size, diff_cmd, name_sq, name_sq);
 	f = popen(cmd, "w");
 	if (old_size)
 		fwrite(old_contents, old_size, 1, f);