From: hno <> Date: Sun, 5 Nov 2000 06:04:09 +0000 (+0000) Subject: Cross-site scripting fixes by Robert Collins and Henrik Nordstrom X-Git-Tag: SQUID_3_0_PRE1~1786 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=10270faab2bc84e50cde13431a6e6d1932966e5f;p=thirdparty%2Fsquid.git Cross-site scripting fixes by Robert Collins and Henrik Nordstrom Everywhere where Squid inserts text received from the network into a HTML page (error pages, FTP listings, Gopher listings, ...) care must be taken to ensure that the text is properly encoded as HTML, or a malicious user might be able to insert script code or other HTML tags, and exploit the web browser of any user visiting their page or clicking on that funny link received in a email.. --- diff --git a/include/util.h b/include/util.h index 232b70b7dc..8e2bfebd2f 100644 --- a/include/util.h +++ b/include/util.h @@ -1,5 +1,5 @@ /* - * $Id: util.h,v 1.55 2000/10/17 08:06:01 adrian Exp $ + * $Id: util.h,v 1.56 2000/11/04 23:04:09 hno Exp $ * * AUTHOR: Harvest Derived * @@ -84,6 +84,9 @@ extern char *rfc1738_escape_unescaped(const char *); extern char *rfc1738_escape_part(const char *); extern void rfc1738_unescape(char *); +/* html.c */ +extern char *html_quote(const char *); + #if XMALLOC_STATISTICS extern void malloc_statistics(void (*)(int, int, int, void *), void *); #endif diff --git a/lib/Makefile.in b/lib/Makefile.in index df1d6bb5d6..10c1686560 100644 --- a/lib/Makefile.in +++ b/lib/Makefile.in @@ -1,5 +1,5 @@ # -# $Id: Makefile.in,v 1.45 1999/07/13 14:51:03 wessels Exp $ +# $Id: Makefile.in,v 1.46 2000/11/04 23:04:09 hno Exp $ # prefix = @prefix@ top_srcdir = @top_srcdir@ @@ -39,6 +39,7 @@ UTILOBJS = rfc1123.o \ Stack.o \ hash.o \ heap.o \ + html.o \ $(LIBOBJS) REGEXOBJS = GNUregex.o DLMALLOCOBJS = dlmalloc.o diff --git a/src/errorpage.cc b/src/errorpage.cc index 33685197d2..a0b8be61ce 100644 --- a/src/errorpage.cc +++ b/src/errorpage.cc @@ -1,6 +1,6 @@ /* - * $Id: errorpage.cc,v 1.154 2000/07/18 06:16:41 wessels Exp $ + * $Id: errorpage.cc,v 1.155 2000/11/04 23:04:10 hno Exp $ * * DEBUG: section 4 Error Generation * AUTHOR: Duane Wessels @@ -423,6 +423,7 @@ errorConvert(char token, ErrorState * err) request_t *r = err->request; static MemBuf mb = MemBufNULL; const char *p = NULL; /* takes priority over mb if set */ + int do_quote = 1; memBufReset(&mb); switch (token) { @@ -524,6 +525,7 @@ errorConvert(char token, ErrorState * err) memBufPrintf(&mb, "%s", sign_mb.buf); memBufClean(&sign_mb); err->page_id = saved_id; + do_quote = 0; } else { /* wow, somebody put %S into ERR_SIGNATURE, stop recursion */ p = "[%S]"; @@ -561,6 +563,8 @@ errorConvert(char token, ErrorState * err) p = mb.buf; /* do not use mb after this assignment! */ assert(p); debug(4, 3) ("errorConvert: %%%c --> '%s'\n", token, p); + if (do_quote) + p = html_quote(p); return p; } diff --git a/src/ftp.cc b/src/ftp.cc index e5c69f3cc9..a1a2f0538a 100644 --- a/src/ftp.cc +++ b/src/ftp.cc @@ -1,6 +1,6 @@ /* - * $Id: ftp.cc,v 1.296 2000/06/27 22:06:01 hno Exp $ + * $Id: ftp.cc,v 1.297 2000/11/04 23:04:10 hno Exp $ * * DEBUG: section 9 File Transfer Protocol (FTP) * AUTHOR: Harvest Derived @@ -354,22 +354,23 @@ ftpListingStart(FtpStateData * ftpState) wordlist *w; char *dirup; int i, j, k; + char *title; storeBuffer(e); storeAppendPrintf(e, "\n", version_string); storeAppendPrintf(e, "\n", mkrfc1123(squid_curtime)); storeAppendPrintf(e, "\n"); storeAppendPrintf(e, "FTP Directory: %s\n", - ftpState->title_url); + html_quote(ftpState->title_url)); storeAppendPrintf(e, "\n"); if (ftpState->flags.use_base) storeAppendPrintf(e, "\n", - ftpState->base_href); + html_quote(ftpState->base_href)); storeAppendPrintf(e, "\n"); if (ftpState->cwd_message) { storeAppendPrintf(e, "
\n");
 	for (w = ftpState->cwd_message; w; w = w->next)
-	    storeAppendPrintf(e, "%s\n", w->key);
+	    storeAppendPrintf(e, "%s\n", html_quote(w->key));
 	storeAppendPrintf(e, "
\n"); storeAppendPrintf(e, "
\n"); wordlistDestroy(&ftpState->cwd_message); @@ -378,25 +379,26 @@ ftpListingStart(FtpStateData * ftpState) storeAppendPrintf(e, "FTP Directory: "); /* "ftp://" == 6 characters */ assert(strlen(ftpState->title_url) >= 6); - for (i = 6, j = 0; ftpState->title_url[i]; j = i) { + title = html_quote(ftpState->title_url); + for (i = 6, j = 0; title[i]; j = i) { storeAppendPrintf(e, "title_url[i], "/"); - if (ftpState->title_url[i] == '/') + i += strcspn(&title[i], "/"); + if (title[i] == '/') i++; for (k = 0; k < i; k++) - storeAppendPrintf(e, "%c", ftpState->title_url[k]); + storeAppendPrintf(e, "%c", title[k]); storeAppendPrintf(e, "\">"); for (k = j; k < i - 1; k++) - storeAppendPrintf(e, "%c", ftpState->title_url[k]); + storeAppendPrintf(e, "%c", title[k]); if (ftpState->title_url[k] != '/') - storeAppendPrintf(e, "%c", ftpState->title_url[k++]); + storeAppendPrintf(e, "%c", title[k++]); storeAppendPrintf(e, ""); if (k < i) - storeAppendPrintf(e, "%c", ftpState->title_url[k++]); + storeAppendPrintf(e, "%c", title[k++]); if (i == j) { /* Error guard, or "assert" */ storeAppendPrintf(e, "ERROR: Failed to parse URL: %s\n", - ftpState->title_url); + html_quote(ftpState->title_url)); debug(9, 0) ("Failed to parse URL: %s\n", ftpState->title_url); break; } @@ -648,7 +650,7 @@ ftpHtmlifyListEntry(char *line, FtpStateData * ftpState) return html; } /* Handle builtin */ - if (!strcmp(line, "")) { + if (strcmp(line, "") == 0) { /* {icon} {text} {link} */ snprintf(icon, 2048, "\"%-6s\"", mimeGetIconURL("internal-dirup"), @@ -717,10 +719,13 @@ ftpHtmlifyListEntry(char *line, FtpStateData * ftpState) mimeGetIconURL("internal-link"), "[LINK]"); /* sometimes there is an 'l' flag, but no "->" link */ - if (parts->link) + if (parts->link) { + char *link2 = xstrdup(html_quote(rfc1738_escape(parts->link))); snprintf(link, 2048, " -> %s", - rfc1738_escape(parts->link), - parts->link); + link2, + html_quote(parts->link)); + safe_free(link2); + } break; case '\0': snprintf(icon, 2048, "\"%-6s\"", @@ -728,7 +733,7 @@ ftpHtmlifyListEntry(char *line, FtpStateData * ftpState) "[UNKNOWN]"); snprintf(chdir, 2048, " ", - rfc1738_escape(parts->name), + rfc1738_escape_part(parts->name), mimeGetIconURL("internal-dir")); break; case '-': @@ -755,13 +760,13 @@ ftpHtmlifyListEntry(char *line, FtpStateData * ftpState) if (parts->type != '\0') { snprintf(html, 8192, "%s %s%s " "%s%8s%s%s%s%s\n", - href, icon, href, text, dots_fill(strlen(text)), + href, icon, href, html_quote(text), dots_fill(strlen(text)), parts->date, size, chdir, view, download, link); } else { /* Plain listing. {icon} {text} ... {chdir}{view}{download} */ snprintf(html, 8192, "%s %s%s " "%s%s%s%s\n", - href, icon, href, text, dots_fill(strlen(text)), + href, icon, href, html_quote(text), dots_fill(strlen(text)), chdir, view, download, link); } ftpListPartsFree(&parts); diff --git a/src/gopher.cc b/src/gopher.cc index 1c7f03a78e..d20b144bff 100644 --- a/src/gopher.cc +++ b/src/gopher.cc @@ -1,6 +1,6 @@ /* - * $Id: gopher.cc,v 1.155 2000/06/27 22:06:02 hno Exp $ + * $Id: gopher.cc,v 1.156 2000/11/04 23:04:10 hno Exp $ * * DEBUG: section 10 Gopher * AUTHOR: Harvest Derived @@ -469,19 +469,20 @@ gopherToHTML(GopherStateData * gopherState, char *inbuf, int len) break; } - memset(tmpbuf, '\0', TEMP_BUF_SIZE); if ((gtype == GOPHER_TELNET) || (gtype == GOPHER_3270)) { if (strlen(escaped_selector) != 0) - snprintf(tmpbuf, TEMP_BUF_SIZE, " %s\n", - icon_url, escaped_selector, host, name); + snprintf(tmpbuf, TEMP_BUF_SIZE, " %s\n", + icon_url, escaped_selector, rfc1738_escape_part(host), + *port ? ":" : "", port, html_quote(name)); else - snprintf(tmpbuf, TEMP_BUF_SIZE, " %s\n", - icon_url, host, name); + snprintf(tmpbuf, TEMP_BUF_SIZE, " %s\n", + icon_url, rfc1738_escape_part(host), *port ? ":" : "", + port, html_quote(name)); } else { snprintf(tmpbuf, TEMP_BUF_SIZE, " %s\n", - icon_url, host, gtype, escaped_selector, name); + icon_url, host, gtype, escaped_selector, html_quote(name)); } safe_free(escaped_selector); strcat(outbuf, tmpbuf); @@ -515,10 +516,10 @@ gopherToHTML(GopherStateData * gopherState, char *inbuf, int len) break; if (gopherState->cso_recno != recno) { - snprintf(tmpbuf, TEMP_BUF_SIZE, "

Record# %d
%s

\n
", recno, result);
+			snprintf(tmpbuf, TEMP_BUF_SIZE, "

Record# %d
%s

\n
", recno, html_quote(result));
 			gopherState->cso_recno = recno;
 		    } else {
-			snprintf(tmpbuf, TEMP_BUF_SIZE, "%s\n", result);
+			snprintf(tmpbuf, TEMP_BUF_SIZE, "%s\n", html_quote(result));
 		    }
 		    strcat(outbuf, tmpbuf);
 		    gopherState->data_in = 1;
@@ -543,7 +544,7 @@ gopherToHTML(GopherStateData * gopherState, char *inbuf, int len)
 		    case 502:	/* Too Many Matches */
 			{
 			    /* Print the message the server returns */
-			    snprintf(tmpbuf, TEMP_BUF_SIZE, "

%s

\n
", result);
+			    snprintf(tmpbuf, TEMP_BUF_SIZE, "

%s

\n
", html_quote(result));
 			    strcat(outbuf, tmpbuf);
 			    gopherState->data_in = 1;
 			    break;