CVE-2012-3501: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: The squidclamav_check_preview_handler function in squidclamav.c in SquidClamav 5.x before 5.8 and 6.x before 6.7 passes an unescaped URL to a system command call, which allows remote attackers to cause a denial of service (daemon crash) via a URL with certain characters, as demonstrated using %0D or %0A.
CWE description: The product performs operations on a memory buffer, but it reads from or writes to a memory location outside the buffer's intended boundary. This may result in read or write operations on unexpected memory locations that could be linked to other variables, data structures, or internal program data.
Published: 2012-08-25T10:29Z
Repository: https://github.com/darold/squidclamav
Language: C
Files changed: src/squidclamav.c
Commit date: 2012-07-24 15:44:54 +0200
Commit message: Add a workaround for a squidGuard bug that unescape the URL and send it back unescaped. This could conduct in wrong result and ssquidclamav crash especially with URL containing the %0D or %0A character. John Xue

Diff: diff --git a/src/squidclamav.c b/src/squidclamav.c
index 7aa63a7..44e614e 100644
--- a/src/squidclamav.c
+++ b/src/squidclamav.c
@@ -131,11 +131,11 @@ void free_pipe ();
 void generate_redirect_page(char *, ci_request_t *, av_req_data_t *);
 void cfgreload_command(char *, int, char **);
 int create_pipe(char *command);
 int dconnect (void);
 int connectINET(char *serverHost, uint16_t serverPort);
-
+char * replace(const char *s, const char *old, const char *new);
 
 /* ----------------------------------------------------- */
 
 
 int squidclamav_init_service(ci_service_xdata_t * srv_xdata,
@@ -363,14 +363,18 @@ int squidclamav_check_preview_handler(char *preview_data, int preview_data_len,
      }
 
      
      /* Check URL header against squidGuard */
      if (usepipe == 1) {
+	char *rbuff = NULL;
 	ci_debug_printf(2, "DEBUG squidclamav_check_preview_handler: Sending request to chained program: %s\n", squidguard);
 	ci_debug_printf(2, "DEBUG squidclamav_check_preview_handler: Request: %s %s %s %s\n", httpinf.url,clientip,username,httpinf.method);
-	fprintf(sgfpw,"%s %s %s %s\n",httpinf.url,clientip,username,httpinf.method);
+	/* escaping escaped character to prevent unescaping by squidguard */
+	rbuff = replace(httpinf.url, "%", "%25");
+	fprintf(sgfpw,"%s %s %s %s\n",rbuff,clientip,username,httpinf.method);
 	fflush(sgfpw);
+	xfree(rbuff);
 	/* the chained redirector must return empty line if ok or the redirection url */
 	chain_ret = (char *)malloc(sizeof(char)*MAX_URL_SIZE);
 	if (chain_ret != NULL) {
 	   ret = fgets(chain_ret,MAX_URL_SIZE,sgfpr);
 	   if ((ret != NULL) && (strlen(chain_ret) > 1)) {
@@ -1626,5 +1630,42 @@ connectINET(char *serverHost, uint16_t serverPort)
 	}
 
         return asockd;
 }
 
+
+/**
+ * Searches all occurrences of old into s
+ * and replaces with new
+ */
+char *
+replace(const char *s, const char *old, const char *new)
+{
+	char *ret;
+	int i, count = 0;
+	size_t newlen = strlen(new);
+	size_t oldlen = strlen(old);
+
+	for (i = 0; s[i] != '\0'; i++) {
+		if (strstr(&s[i], old) == &s[i]) {
+			count++;
+			i += oldlen - 1;
+		}
+	}
+	ret = malloc(i + 1 + count * (newlen - oldlen));
+	if (ret != NULL) {
+		i = 0;
+		while (*s) {
+			if (strstr(s, old) == s) {
+				strcpy(&ret[i], new);
+				i += newlen;
+				s += oldlen;
+			} else {
+				ret[i++] = *s++;
+			}
+		}
+		ret[i] = '\0';
+	}
+
+	return ret;
+}
+

Vulnerable code: 
	fprintf(sgfpw,"%s %s %s %s\n",httpinf.url,clientip,username,httpinf.method);
Fixed code: char * replace(const char *s, const char *old, const char *new);
	char *rbuff = NULL;
	/* escaping escaped character to prevent unescaping by squidguard */
	rbuff = replace(httpinf.url, "%", "%25");
	fprintf(sgfpw,"%s %s %s %s\n",rbuff,clientip,username,httpinf.method);
	xfree(rbuff);

/**
 * Searches all occurrences of old into s
 * and replaces with new
 */
char *
replace(const char *s, const char *old, const char *new)
{
	char *ret;
	int i, count = 0;
	size_t newlen = strlen(new);
	size_t oldlen = strlen(old);

	for (i = 0; s[i] != '\0'; i++) {
		if (strstr(&s[i], old) == &s[i]) {
			count++;
			i += oldlen - 1;
		}
	}
	ret = malloc(i + 1 + count * (newlen - oldlen));
	if (ret != NULL) {
		i = 0;
		while (*s) {
			if (strstr(s, old) == s) {
				strcpy(&ret[i], new);
				i += newlen;
				s += oldlen;
			} else {
				ret[i++] = *s++;
			}
		}
		ret[i] = '\0';
	}

	return ret;
}