49021 - Parse incoming HTML msgs (Hack-Security)

Status: VERIFIED WONTFIX

(MailNews Core :: MIME, enhancement, P3)

Description

[Ben Bucksch (:BenB)]

I have a huge hack in my tree where I search for common threats in incomming
HTML msgs, e.g. external imgs, stylesheets, scripts etc., and remove or replace
them (so I can add a big warning per stylesheet).

If there is interest, I can add a pref (default off) for that and check it in,
assuming OK from relevant powers.

Note: This does not provide real security. I might have missed a threat. Also,
parsing the HTML source is a hack, if you compare this with the flexibility and
power the rendering engine provides (or can provide) for security. See bug 28327
for real (I hope) security.

Comment 1

[John G. Myers]

I don't think it's worth spending time on such kludges.

Comment 2

[Brendan Eich [:brendan]]

Why not drop this kludgy idea, and work on bug 37983?

/be

Comment 3

[Ben Bucksch (:BenB)]

If you say so...

WONTFIX.

Comment 4

[Ben Bucksch (:BenB)]

It turns out that the bugfix would have been useful for other bug, bug 66938.
Unfortunately, I lost the change locally and didn't attach it. So, I'll roughly
describe what I did.

The HTML msgs run though libmime. In the content-handling function of the HTML
msg handler, which currently does almost nothing than just copying, I did a
search and replace for offending tags and replaced them with harmless ones. E.g.
I substituted. |<script| with |<span style="display: none" class="script"| or
|onclick| with |nil|* and similar. *(On a second thought, what did I do, if
|onclick| appeared in the content? I don't remember.)
To make my life easier, I first converted the char* to a nsString, which
provides nice replace functions.

Comment 5

[Ben Bucksch (:BenB)]

Forgot the code-reference: function |MimeInlineTextHTML_parse_line|.

Comment 6

[Ben Bucksch (:BenB)]

> *(On a second thought, what did I do, if |onclick| appeared in the content? I
> don't remember.)

We could check, if an open "<" is before it.

If we are talking about enabling this "hack" per default, we need to care about
holes a lot more.
Are there any cases constructable, where a |line.Find("<script"...)| would fail
to find the <script> tag?

Comment 7

[fenella]

Mark it verified.

Comment 8

[Ben Bucksch (:BenB)]

Bug 108153 is the followup to this one and uses the main Mozilla parser, i.e. is
not a hack.