Closed Bug 504965 Opened 15 years ago Closed 6 years ago

Provide tests for javascript execution on RSS feeds

Categories

(Thunderbird :: Security, defect)

x86
Windows XP
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: JoeS1, Unassigned)

References

()

Details

Attachments

(3 files)

Subscribe to the feed mentioned in the url
Locate the post "an overview of TraceMonkey"
The sample scripts you see there will execute.

[Personally, I think it was a mistake to disable JS without a pref to selectively enable, so if someone would like to email me a one-liner change in nsMsgContentPolicy.cpp as a reward for exercising due diligence here, that would be great.]

Noticed in:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.1pre) Gecko/20090717 Lightning/1.0pre Shredder/3.0b4pre ID:20090717034003
More reproducible detail, please.

Since Planet strips the <script type="text/javascript"> from the example scripts, and the onclick attribute from the <button>, it would be *very* surprising to see them run from http://planet.mozilla.org/atom.xml

I also don't see them run from http://hacks.mozilla.org/feed/atom/, but at least there there's some sort of possibility that some combination of way of subscribing and prefs and maybe addons could cause them to run, since at least there there's actual script that could run.
Status: NEW → UNCONFIRMED
Ever confirmed: false
Attached image screenshot
Dunno Phil
I do have javascript.allow.mailnews set
and the tracemonkey content pref set.
Nothin else as far as caps prefs, as I think they don't work anymore.
Maybe since I have "Feed properties" set to display as a web page and not a summary.
AFAICT javascript.allow.mailnews pref is no longer respected and probably should just be removed - I'll leave Dan to confirm that.

WRT to the feed - I can confirm the feed summary doesn't have javascript in it as Phil said. When loading it as a web page, then yes we are accessing it in a web content and allowing javascript.

If you read Dan's original comments on the matter in bug 453928 comment 26:

> I believe that there are valid use cases surrounding JS web-pages pointed to by
> feed summaries.  For tb3, they will continue to be loaded in an iframe inside a
> message, so they will continue to have JS disabled (as in tb2).  Sometime down
> the road, we should change the feed code to load those in more web-like
> contexts.  In the tb3 timeframe, someone could do this in an extension, I
> suspect.

The iframe change has now happened in a different bug, so the statement saying it will stay the say for tb3 is no longer true. If I look in DOMI the contentDocument.location is http://hacks.mozilla.org/2009/07/tracemonkey-overview/ which means the browser element is loading in the web page direct, not wrapped in an iframe.
What is confusing here, is that if you view message source, you see the sanitized version of the post, with onclick() on the button removed.
But that's not what you are really viewing. What you see, is not what you get.
(seems to me there was an extension that would extend view source to include the remote content ??) Anyhow, if you drill down to the actual content with domi you will see:
function onclick(event) {
    runExample();
}

IMO I think this behavior is fine, but a bit magical to the average user.

If this is considered acceptable, known behavior, I'll mark this INVALID

Waiting for comment from dmose, for that resolution.
(In reply to comment #6)
> AFAICT javascript.allow.mailnews pref is no longer respected and probably
> should just be removed - I'll leave Dan to confirm that.

Correct.

(In reply to comment #6)
> The iframe change has now happened in a different bug, so the statement saying
> it will stay the say for tb3 is no longer true. If I look in DOMI the
> contentDocument.location is
> http://hacks.mozilla.org/2009/07/tracemonkey-overview/ which means the browser
> element is loading in the web page direct, not wrapped in an iframe.

Interesting; in which bug did that change?  I'd like to have a look at that code...
This blocks Tb3 in the sense that we need to at least look at it in more detail before deciding what to do.  As quoted from the previous bug, it's a valid use case.  Whether the removal of the iframe really makes much of a security difference here depends on the details of what it got replaced with.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking-thunderbird3+
(In reply to comment #8)


> > element is loading in the web page direct, not wrapped in an iframe.
> 
> Interesting; in which bug did that change?  I'd like to have a look at that
> code...

I believe that happened in bug 497895
I remember that I was concerned that we would actually loose some functionality as a result. iframe seemed to allow acid tests to work more fully in a message context.
I found no ill-effects on feeds after the iframe change.
(In reply to comment #10)
> (In reply to comment #8)
> 
> 
> > > element is loading in the web page direct, not wrapped in an iframe.
> > 
> > Interesting; in which bug did that change?  I'd like to have a look at that
> > code...
> 
> I believe that happened in bug 497895

I think it is worth adding that bug 438429 made some initial changes, then bug 497895 followed up.
Yeah bug 497895 only got rid of an iframe that wasn't used for anything.
Whiteboard: [no l10n impact]
Target Milestone: --- → Thunderbird 3.0b4
Dan and I have just been discussing this bug, we've tried a few things and haven't found any issues:

1) Viewing as web page the feed item is loaded via a http:// protocol and therefore the content policies treats it as regular content.
2) Viewing as summary the feed item is loaded via a mailbox:// protocol and therefore javascript is disabled (this is additionally proved by the mozmill test test-js-content-policy.js, though could be extended explicitly for rss feeds).
3) Forwarding an rss feed item only forwards a link to the item, not the item itself.
4) If you take a feed item that has javascript in its summary, and drag and drop to attach it to a new message, then the attached message will have javascript in it. However this isn't executed when received by Thunderbird, essentially Thunderbird is just a carrier (in the same was as forwarding/attaching an email with script in it could happen), so we don't see this as an issue.

Therefore we think there isn't an issue with js execution in feeds here and hence this isn't a blocker.

However, we would like to get some more automated tests to support the security protocol which would cover at least:

- Checking js execution in an rss feed item shown as a web page and as a summary (obviously web page will work, summary won't).
- Checking forwarding of an rss feed item.
Flags: blocking-thunderbird3+ → blocking-thunderbird3-
Summary: Javascript execution is possible on some RSS feeds → Provide tests for javascript execution on RSS feeds
Whiteboard: [no l10n impact]
Target Milestone: Thunderbird 3.0b4 → ---
test-js-content-policy.js has tests for javascript in feeds nowadays. Bug 662907, bug 1151497.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.