[PATCH] Fix double-free crash if EOF immediately follows </MAP>.

Kalle Olavi Niemitalo <[email protected]> Mon, 29 Dec 2008 21:49:52 +0200
Newsgroups gmane.comp.web.links
Message-ID <[email protected]>
look_for_link() used to return 0 both when it found the closing </MAP>
tag, and when it hit the end of the file.  In the first case, it also
added *menu to the memory_list; in the second case, it did not.  The
caller get_image_map() supposedly distinguished between these cases by
checking whether pos >= eof, and freed *menu separately if so.

However, if the </MAP> was at the very end of the HTML file, so that
not even a newline followed it, then look_for_link() left pos == eof
even though it had found the </MAP> and added *menu to the memory_list.
This made get_image_map() misinterpret the result and mem_free(*menu)
even though *menu had already been freed as part of the memory_list;
thus the crash.

To fix this, make look_for_link() return -1 instead of 0 if it hits
EOF without finding the </MAP>.  Then make get_image_map() check the
return value instead of comparing pos to eof.

Alternatively, look_for_link() could have been changed to decrement
pos between finding the </MAP> and returning 0.  Then, the pos >= eof
comparison in get_image_map() would have been false.  That scheme
would however have been a bit more difficult to understand and
maintain, I think.

Reported by Paul B. Mahol.
---
 NEWS                       |    1 +
 src/document/html/parser.c |   17 ++++++++++++-----
 test/imgmap2.html          |    5 +++++
 3 files changed, 18 insertions(+), 5 deletions(-)
 create mode 100644 test/imgmap2.html

diff --git a/NEWS b/NEWS
index ab36a8e..ebd7c80 100644
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,7 @@ ELinks 0.12pre2.GIT now:
 To be released as 0.12pre3, 0.12rc1, or even 0.12.0.  This branch also
 includes the changes listed under ``ELinks 0.11.5.GIT'' below.
 
+* critical: Fix double-free crash if EOF immediately follows </MAP>.
 * Preserve newlines in hidden input fields, and submit them as CRLF.
   Previously, they could turn into spaces or disappear entirely.
 * Perl scripts can use modules that dynamically load C libraries, like
diff --git a/src/document/html/parser.c b/src/document/html/parser.c
index 4d8c111..d9e911a 100644
--- a/src/document/html/parser.c
+++ b/src/document/html/parser.c
@@ -615,6 +615,9 @@ look_for_tag(unsigned char **pos, unsigned char *eof,
 	return 0;
 }
 
+/** @return -1 if EOF is hit without the closing tag; 0 if the closing
+ * tag is found (in which case this also adds *@a menu to *@a ml); or
+ * 1 if this should be called again.  */
 static int
 look_for_link(unsigned char **pos, unsigned char *eof, struct menu_item **menu,
 	      struct memory_list **ml, struct uri *href_base,
@@ -632,7 +635,7 @@ look_for_link(unsigned char **pos, unsigned char *eof, struct menu_item **menu,
 		(*pos)++;
 	}
 
-	if (*pos >= eof) return 0;
+	if (*pos >= eof) return -1;
 
 	if (*pos + 2 <= eof && ((*pos)[1] == '!' || (*pos)[1] == '?')) {
 		*pos = skip_comment(*pos, eof);
@@ -647,7 +650,7 @@ look_for_link(unsigned char **pos, unsigned char *eof, struct menu_item **menu,
 	if (!c_strlcasecmp(name, namelen, "A", 1)) {
 		while (look_for_tag(pos, eof, name, namelen, &label));
 
-		if (*pos >= eof) return 0;
+		if (*pos >= eof) return -1;
 
 	} else if (!c_strlcasecmp(name, namelen, "AREA", 4)) {
 		/* FIXME (bug 784): options->cp is the terminal charset;
@@ -765,6 +768,7 @@ get_image_map(unsigned char *head, unsigned char *pos, unsigned char *eof,
 {
 	struct conv_table *ct;
 	struct string hd;
+	int look_result;
 
 	if (!init_string(&hd)) return -1;
 
@@ -785,10 +789,13 @@ get_image_map(unsigned char *head, unsigned char *pos, unsigned char *eof,
 
 	*ml = NULL;
 
-	while (look_for_link(&pos, eof, menu, ml, uri, target_base, ct, options))
-		;
+	do {
+		/* This call can modify both *ml and *menu.  */
+		look_result = look_for_link(&pos, eof, menu, ml, uri,
+					    target_base, ct, options);
+	} while (look_result > 0);
 
-	if (pos >= eof) {
+	if (look_result < 0) {
 		freeml(*ml);
 		mem_free(*menu);
 		return -1;
diff --git a/test/imgmap2.html b/test/imgmap2.html
new file mode 100644
index 0000000..e96e184
--- /dev/null
+++ b/test/imgmap2.html
@@ -0,0 +1,5 @@
+<TITLE>Double-free crash in USEMAP</TITLE>
+<P><IMG src="/dev/null" usemap="#crasher"></P>
+<MAP name="crasher">
+<AREA shape="rect" coords="42,42,69,69" href="http://elinks.cz/" alt="see this?">
+<!-- no newline at the end of this line --></MAP>
\ No newline at end of file
-- 
1.6.0.6.4.g22c5

_______________________________________________
elinks-dev mailing list
[email protected]
http://linuxfromscratch.org/mailman/listinfo/elinks-dev
signature.asc (application/pgp-signature, 188 B)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFJWSnqHm9IGt60eMgRAqMHAKCgBFT+C3TO7KHPtWMH6sw9i53f6gCgi+fj
8xsezv6Lz5tjtmMavcQuuNA=
=zYX6
-----END PGP SIGNATURE-----