Persona is no longer an option for authentication on BMO. For more details see Persona Deprecated.
Last Comment Bug 519252 - consider builder newsgroup link in a safer way
: consider builder newsgroup link in a safer way
Product: Thunderbird
Classification: Client Software
Component: Mail Window Front End (show other bugs)
: 3.0
: All All
: -- normal (vote)
: Thunderbird 22.0
Assigned To: Magnus Melin
Depends on:
  Show dependency treegraph
Reported: 2009-09-28 11:58 PDT by Magnus Melin
Modified: 2013-03-25 12:31 PDT (History)
2 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

proposed fix (1.96 KB, patch)
2013-03-23 04:57 PDT, Magnus Melin
standard8: review+
Details | Diff | Splinter Review

Description Magnus Melin 2009-09-28 11:58:09 PDT
+++ 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 2013-03-23 04:57:42 PDT
Created attachment 728608 [details] [diff] [review]
proposed fix

I guess this would be the standard way to make sure the url is ok.
Comment 2 Mark Banner (:standard8) 2013-03-25 08:03:44 PDT
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 2013-03-25 12:31:52 PDT -> FIXED

Note You need to log in before you can comment on or make changes to this bug.