It appears that the 'background' attribute of at least certain tags (td in particular, and almost certainly others) is not being stripped / filtered out of the code like the src, lowsrc, and href attributes typically are. According to the HTML specs this is not a legal attribute to begin with (which likely explains why it's not currently on the filtered list), but several browsers support its use anyway. So IMO it needs to be handled if possible.
Trivial to drop it from all tags.
Also, when investigating this issue, I started considering what else might have been missed in the current design of the filtering code. So, I tried sending myself a few messages with CSS-defined images. It seems that nothing is done about style tags nor style attributes contained inside other tags. CSS can specify certain properties with url() definitions -- properties such as background-image, list-style, background, etc. -- causing the message to load an image, contrary to expectations. Furthermore, the @import rule in a style tag can cause an external file to be requested as well.
That's all actually fairly understandable, since filtering specific CSS declarations out of a particular attribute (or the content of a tag) might be difficult in a code design which currently seems (at least apparently) to treat HTML attribute values (and tag contents) basically as atoms -- i.e. to be discarded as a whole, or used in its entirety as a portion of a redirected URL. But I think something should be done to remove any url() values from CSS declarations. And likewise, in the case of style tags, I think any @import rules should also be removed -- and possibly from any style attributes, in case any browsers improperly allow @import rules inside style attributes.
I think I'll just nuke the STYLE attribute on all tags.
Everything in <STYLE> </STYLE> is already filtered out.
P.S. Sam... Of course, I *am* going to try to work up a patch for at least the first issue (and hopefully the second one) for you myself. But if you can get it done fairly quickly, I certainly wouldn't hold my breath waiting on me, if I were you. :) I mean, I might get to it and be done quickly, but then again I might not. -- And you certainly know the code better than I do after all.
Yes. The patch is trivial. Try this:
pgp00000.pgp
Description: PGP signature
Index: sqwebmail/html.c
===================================================================
RCS file: /cvsroot/courier/courier/webmail/html.c,v
retrieving revision 1.18
diff -U3 -r1.18 html.c
--- sqwebmail/html.c 17 Jun 2003 15:38:37 -0000 1.18
+++ sqwebmail/html.c 5 Oct 2003 23:50:05 -0000
@@ -86,12 +86,14 @@
The ONLOAD, ONMOUSEOVER, and all other ON* attributes are removed.
- Attributes TARGET, CODE, ACTION, CODETYPE and LANGUAGE are removed.
+ Attributes BACKGROUND, STYLE, TARGET, CODE, ACTION, CODETYPE and
+ LANGUAGE are removed.
TARGET=_blank is added to all <A> tags.
HREF, SRC, or LOWSRC attributes that do not specify a URL of http:,
https:, ftp:, gopher:, wais:, or telnet:, are removed.
+ Everything in <STYLE> </STYLE> is dropped..
*/
char *tagbuf=0;
@@ -705,6 +707,8 @@
is_attr(tagattr+l, "code") ||
is_attr(tagattr+l, "language") ||
is_attr(tagattr+l, "action") ||
+ is_attr(tagattr+l, "background") ||
+ is_attr(tagattr+l, "style") ||
(is_attr(tagattr+l, "type")
&& !is_htmltag("BLOCKQUOTE")) ||
is_attr(tagattr+l, "codetype"))
pgp00001.pgp
Description: PGP signature
