519252 - consider builder newsgroup link in a safer way

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Magnus Melin [:mkmelin]]

+++ This bug was initially created as a clone of Bug #519118 comment 2 +++

In general, this looks good.  However, from a security standpoint, Newsgroups
originally comes from an untrusted source (the message).  Now, if there are no
bugs in the rest of our code, we'll be fine constructing the URL using strings
like this.  However, it's better to have defense in depth.  Ideally, we'd build
up the URL object using nsIURI attribute setters on an nsStandardURL object so
that it enforces all the right escaping.  Assuming nsStandardURL supports that
mode of use, please do that, and request review from again.  If not, please
file a bug on nsStandardURL and we'll live with the string construction for
now, so r+ing now for that case.

Comment 1

[Magnus Melin [:mkmelin]]

Attachment: proposed fix

I guess this would be the standard way to make sure the url is ok.

Comment 2

[Mark Banner (:standard8)]

Comment on attachment 728608 [details] [diff] [review]
proposed fix

Review of attachment 728608 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/base/content/msgHdrViewOverlay.js
@@ +1725,3 @@
>    let url;
>    if (server.socketType != Components.interfaces.nsMsgSocketType.SSL) {
>      url = "news://" + server.hostName;

This lot feels like it is begging for a utility function, unfortunately I don't think we have one, and we wouldn't be able to share it easily anyway (due to historical mailnews issues). Oh well.

Comment 3

[Magnus Melin [:mkmelin]]

https://hg.mozilla.org/comm-central/rev/b411f5facb9b -> FIXED