240262 - Marquee tag functions only in default mode

Status: RESOLVED FIXED

(MailNews Core :: Security, defect)

Description

[Joe Sabash [:JoeS1]]

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.6a) Gecko/20031001 Firebird/0.7+ (aebrahim)
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.6a) Gecko/20031001 Firebird/0.7+ (aebrahim)

If 'behavior' or 'direction' attributes are specified the marquee becomes
inoperative 

Reproducible: Always
Steps to Reproduce:
1.see attachment below
2.
3.

Actual Results:  
Marquee tag is ignored

Expected Results:  
Marquee should execute with specified behavior attributes

Comment 1

[Joe Sabash [:JoeS1]]

Attachment: Horizontal marquee

This sample works fine in the browser
Copy the body contents into a mail composition, and view the results

Comment 2

[Joe Sabash [:JoeS1]]

Attachment: Vertical Marquee with javascript references

This example has an additional twist, and that is a JS start/stop call
Works fine in the browser

Comment 3

[Joe Sabash [:JoeS1]]

Not sure when this JS error came into play, but at some point:
Error: uncaught exception: [Exception... "Component returned failure code:
0x80004005 (NS_ERROR_FAILURE) [nsIObserverService.removeObserver]"  nsresult:
"0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame ::
chrome://global/content/bindings/browser.xml :: destroy :: line 484"  data: no]
For what it's worth

Comment 4

[Doron Rosenberg (IBM)]

So you are trying to compose a marquee in tbird? Evil.  What version of tbird
are you using?  Not sure if its forked, as the trunk got some marquee love last
week.

Comment 5

[Joe Sabash [:JoeS1]]

Attachment: script controlled marquee

Comment 6

[Joe Sabash [:JoeS1]]

The marquee code seems to be much more efficient than using DHTML to do the
same thing. Much less CPU intensive, especially with images.
My primary interest is in Graphics Newsgroup posting, not normal mail.
I assume that the bindings in the style, are allowing this to work, however
the same bindings do not enable the features without the use of script as in
the attachment.
It would be nice if a general style declaration would enable fullfunctions in
normal mail.
At least until we can persuade someone to enable full marquee function in mail.
OH yes, I am using a build which includes the 04/07 checkin

Comment 7

[Joe Sabash [:JoeS1]]

I am not sure now that the -moz-binding had anything to do with the
functionality. It seems Tbird does not initiate the marquee reliably.
Another curious aspect of this is if I bring up the DOM inspector, then
Inspect  URL snews://secnews.netscape.com:563/c5nd6i%24o8512%40ripley.netscape.com
The browser window runs the vertical direction fine, even though my browser
does Not have the new XBL features.

Comment 8

[Scott MacGregor]

Joe are you testing branch builds or trunk builds? The branch builds were cut
long before these potential marquee changes went into the trunk I suspect. 

Comment 9

[Joe Sabash [:JoeS1]]

I'm using a trunk build dated 2 days after Doron's changes.
To be sure I unpacked the Mail.jar and looked.
I guess I should clarify the problem a bit.
Prior to the changes Thunderbird and Moz mail/news would execute the marquee
only in default mode. If a behavior or direction was specified in the html tag
the marquee would not start at all (no javascript errors in console.)
My attempt to use the new marquee features was to see if this method of starting
the marquee would allow using the behavior and direction attributes in News.
They do in fact function (except for the up/down controls)
All of the behaviors in the first two attachments work fine in
rv:1.6a) Gecko/20031001 Firebird/0.7+
So we can use script to start the marquee, but not a simple html marquee tag
If we could get browser marquee functionality in News it could simplify a lot
of things we are now doing totally with Javascript 

Comment 10

[Scott MacGregor]

I wonder if this is just a build/packaging issue where I'm forgetting to package
something xbl-marquee needs or is. 

Comment 11

[Joe Sabash [:JoeS1]]

Attachment: Simplified 'right' marquee

Marquee tag with right direction specified

Comment 12

[Joe Sabash [:JoeS1]]

Attachment: DOM I with browser

Javascript object properties (browser view)

Comment 13

[Joe Sabash [:JoeS1]]

Attachment: DOM I view in Mail

Javascript properties in Mail

Comment 14

[Joe Sabash [:JoeS1]]

Sorry about the images to make my point (can't seem to copy paste text
out of the DOM inspector window)
From the results in the attachments, seems like Marquee is never initiated
in Mail if there is a direction or behavior attribute

Comment 15

[Doug Wright]

Confirmed with Tb 1.0

Comment 16

[Mike Cowperthwaite]

(In reply to comment #15)
> Confirmed with Tb 1.0

What exactly are you confirming?  I tried pasting attachment 145895 [details] into a mail 
message, and I don't see any marquee behavior, including the "default" -- all 
the table entries are blank, not simply inoperative.  (TB 1.0, Win2K)  I don't 
know if that's what reporter meant by "Marquee tag is ignored," but I doubt it.

FWIW, that same HTML in Firefox 1.0, the "slide" behavior does not operate as 
described; instead, it appears to act just as the default behavior does.

Comment 17

[Doug Wright]

I pasted the attachment into the message compose window, and only the first
marquee (with no attributes) showed up (although it wasn't scrolling) - the
other marquees (with attributes) were hidden completely.

That's Tb 1.0 on WinXP

Comment 18

[Joe Sabash [:JoeS1]]

(In reply to comment #16)
> (In reply to comment #15)
> > Confirmed with Tb 1.0
> 
> What exactly are you confirming?  I tried pasting attachment 145895 [details] [edit]
into a mail 
> message, and I don't see any marquee behavior, including the "default" -- all 
> the table entries are blank, not simply inoperative.  (TB 1.0, Win2K)  I don't 
> know if that's what reporter meant by "Marquee tag is ignored," but I doubt it.
> 
> FWIW, that same HTML in Firefox 1.0, the "slide" behavior does not operate as 
> described; instead, it appears to act just as the default behavior does.

All five of those marquees work in Mozilla/5.0 (Windows; U; Windows NT 5.1;
en-US; rv:1.8a6) Gecko/20041224 Firefox/1.0+ here as well as the release build.
If you are not seeing them, check your prefs, there is one somewhere that blocks
the marquee function.

Comment 19

[Joe Sabash [:JoeS1]]

(In reply to comment #17)
> I pasted the attachment into the message compose window, and only the first
> marquee (with no attributes) showed up (although it wasn't scrolling) - the
> other marquees (with attributes) were hidden completely.
> 
> That's Tb 1.0 on WinXP
I'm not sure pasting into the compose window is foolproof, what might be better
is using insert| html, then pasting in the code. At any rate,marquee does not
start while in the compose window, either send it to yourself or, use 'send
later' then open it from the unsent folder. 

Comment 20

[Doug Wright]

(In reply to comment #19)
> I'm not sure pasting into the compose window is foolproof, what might be better
> is using insert| html, then pasting in the code.

That's what I meant/did.

Comment 21

[Mike Cowperthwaite]

(In reply to comment #18)
> (In reply to comment #16)
> > I tried pasting attachment 145895 [details] into a mail 
> > message, and I don't see any marquee behavior, including the "default" -- 
> > all the table entries are blank, not simply inoperative.  (TB 1.0, Win2K)
> >
> > FWIW, that same HTML in Firefox 1.0, the "slide" behavior does not operate
> > as described; instead, it appears to act just as the default behavior does.
> 
> All five of those marquees work in ... Firefox/1.0+ here as well as the
> release build.  If you are not seeing them, check your prefs, there is one
> somewhere that blocks the marquee function.

Not sure which part of my comment you're responding to.  I *am* seeing the tags, 
scrolling, in Firefox, also in Moz 1.8a6; it's in TB, and in Moz MailNews, that 
they're coming up blank.  When you reported "Marquee tag is ignored" did you 
mean it showed blank, or that it showed the text without scrolling?

Comment 22

[Doug Wright]

> What exactly are you confirming?  I tried pasting attachment 145895 [details] [edit]
into a mail 
> message, and I don't see any marquee behavior, including the "default" -- all 
> the table entries are blank, not simply inoperative.  (TB 1.0, Win2K)  I don't 
> know if that's what reporter meant by "Marquee tag is ignored," but I doubt it.
Sending the message to myself, gives me the quoted behaviour.
 
> FWIW, that same HTML in Firefox 1.0, the "slide" behavior does not operate as 
> described; instead, it appears to act just as the default behavior does.
Same here

Comment 23

[Joe Sabash [:JoeS1]]

(In reply to comment #21)
> (In reply to comment #18)
> > (In reply to comment #16)
> 
> Not sure which part of my comment you're responding to.  I *am* seeing the tags, 
> scrolling, in Firefox, also in Moz 1.8a6; it's in TB, and in Moz MailNews, that 
> they're coming up blank.  When you reported "Marquee tag is ignored" did you 
> mean it showed blank, or that it showed the text without scrolling?

No marquee, no text, blank boxes. I guess you could consider that 'data loss' if
you wanted to push the definition. In regard to the 'slide behavior' I don't
think the XBL implimentation ever intended to completely emulate MS

Comment 24

[Joe Sabash [:JoeS1]]

Oh, I think I see the confusion factor here. Javascript must be enabled in
Mail/News to see the default marquee at all.

Comment 25

[Martijn Wargers (dead)]

Ok, I can see the bug. 
Using a normal marquee works, but using a marquee behavior=etc doesn't work at all. I get a js error in the js console:
Error: uncaught exception: Permission denied to call method HTMLDivElement.getAttribute

When I make a simple draft message, with this pasted in it:
<div id="t" direction="test">t</div>
<script>
function doe(){
alert(document.getElementById('t').getAttribute('direction'));
}
</script>
<button onclick="doe()">doe</button>
I get the same js error.
I don't get the error, when using (although that doesn't work):
alert(document.getElementById('t').direction);

Comment 26

[Martijn Wargers (dead)]

Attachment: message I tested with

This is the mail message I tested with.
With the first button (alert(div.getAttribute(id)) I get the js permission denied error, with the second (alert(div.id)) not.
I can see this bug even in Mozilla1.0RC3, so not a recent regression.

Comment 27

[Martijn Wargers (dead)]

Attachment: patch

so this happens because of the js restriction prefs in all.js.
This patch fixes it by lifting the getAttribute restrictions for HTMLDivElements. I don't think that would cause security problems, since <div>'s (or marquees) don't have 'dangerous' attributes.

Thanks to Joe for testing the pref.

Comment 28

[Martijn Wargers (dead)]

Ok, thanks Scott, now does it need supper-review or can it be checked in?

Comment 29

[Scott MacGregor]

you can check it into the trunk.

Comment 30

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked in.

Comment 31

[Joe Sabash [:JoeS1]]

This patch has been applied to the trunk for some 7 months with no bad effects.
For Newsgroup readers who use Marquee this would amount to a large capability enhancement. I propose that is is time to add this to Thunderbird/Seamonkey branch.
The capabilty can be added easily with CAPS or a specific user.js but for the casual user,the impression is "Mozilla does not fully support the marquee tag in Mail/News"

Comment 32

[Martijn Wargers (dead)]

Comment on attachment 201264 [details] [diff] [review]
patch

Joe, not sure why you're changing all these things, but you should not remove the r+.
I'll ask approval1.81? on the patch (I'm afraid I'm a little bit late with asking for that, though).

Comment 33

[Joe Sabash [:JoeS1]]

My intention was to get the problem addressed as a core issue, so that Seamonkey might get fixed in the process. The R+ was removed unintentionally.
Thanks martijn

Comment 34

[Mike Schroepfer]

Are there any possible security problems as a result of loosening the security policy?

Comment 35

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

What was the rationale for the getAttribute restriction in the first place?

Comment 36

[Martijn Wargers (dead)]

It was added in bug 84545. There is no mention of why they restricted getAttribute.
Maybe for <a href=""> and <img> elements that have src and href attributes. But since marquee doesn't have these, I think this is safe.

Comment 37

[Daniel Veditz [:dveditz]]

bug 84545 (mail "wiretap" exploits) was about the ability of forwarded or replied-to email to snoop on the newly-added contents. <div> attributes should be uninteresting enough, and people are unlikely to add a <div> when composing a mail reply (as long as we don't use similar logic to add most everything back one-by-one).

Jesse: do you concur?

r=dveditz

Comment 38

[Daniel Veditz [:dveditz]]

I should point out that these cases require JS turned on in mail which we discourage in the strongest terms (one 0-day and you've got a raging worm on your hands -- at least in the browser victims have to visit the attacker's site which limits the spread of attacks).

Comment 39

[Jesse Ruderman]

Sounds reasonable.  The only HTML elements that give you an HTMLDivElement seem to be <div>, <marquee>, and <noscript>, and none of those tends to have interesting attributes.

Comment 40

[Joe Sabash [:JoeS1]]

I don't think that js can run in the composition window see bug #121171
So how can the "wiretap" exploit still be an issue at all? The JS capabilities
that are blocked, are commonly used in "stationary" Newsgroups, that use JS for
effects. At any rate, can this seemingly harmless restriction be lifted in core,
by checking in this patch.

 

Comment 41

[Scott MacGregor]

Comment on attachment 201264 [details] [diff] [review]
patch

approving for thunderbird2, but since this changes a core default JS file used by Firefox, I'll ask the 1.8.1.2 triage team for approval too. This pref is only used by mailnews and it's been baking for quite some time.

Comment 42

[Jay Patel [:jay]]

Comment on attachment 201264 [details] [diff] [review]
patch

Approved for 1.8 branch, a=jay for drivers.  Thanks for checking with us Scott, let's get this in.