54703 - adding of new quips could use a few improvements

Status: VERIFIED WONTFIX

(Bugzilla :: Bugzilla-General, enhancement, P3)

Description

[Adam Spiers]

The following patch

  - adds a reset button to newquip.html
  - makes new_comment.cgi use CGI.pl
  - safely allows HTML in quips by using html_quote()
  - checks whether adding the quip succeeded (check those return values!)
  - outputs the standard bugzilla footer

Comment 1

[Adam Spiers]

Attachment: improvements to adding quips

Comment 2

[Adam Spiers]

Argh, that patch had a stupid thinko which reintroduced allowing raw HTML in
quips.  Try this one instead.

Comment 3

[Adam Spiers]

Attachment: working improvements to adding quips

Comment 4

[Ben Bucksch (:BenB)]

What does html_quote do? "<" -> "&lt;" etc.?

Comment 5

[Jesse Ruderman]

I think it's better to tell users that html isn't allowed in new quips than to 
automatically escape it.  And IMO reset buttons are evil.

Comment 6

[Adam Spiers]

Why do you think it's better to refuse html?  That just needlessly restricts
what the quips can contain.

And why do you think reset buttons are evil?  Would you advocate removal
of all reset buttons from bugzilla, or just here?

Comment 7

[Ben Bucksch (:BenB)]

> Why do you think it's better to refuse html?

For security reasons: HTML can contain JS, iframes, images with cookie and all
kinds of other evil stuff.

Yes, that's why we "face" on the web all day, too, but the mozilla.org domain is
also a testbed for Mozilla and since development version can contains all kinds
of seurity bugs, it is essential that the mozilla.org domain is trustworthy. I
wouldn't use early Mozilla versions on the bad sides of the net.

Imaging, there is a known bug in Mozilla. An attacker could just place an
exploit in a quip and get a very high hit rate.

Comment 8

[Adam Spiers]

> > Why do you think it's better to refuse html?
>
> For security reasons: HTML can contain JS, iframes, images with cookie and all
> kinds of other evil stuff.

But escaped HTML can't.

Comment 9

[Ben Bucksch (:BenB)]

That (escape it) is what we do today, not?

Comment 10

[Adam Spiers]

No, currently anything with < in is refused.

Comment 11

[Jesse Ruderman]

The problem is that there are already quite a few quips in the database that do 
contain HTML.  People will see that, and start submitting new quips with HTML 
tags in them, without realizing that their quips are being escaped.

If you add a note to the "add quip" page about escaping, though, it would 
probably be ok.

Comment 12

[Jacob Steenhagen]

I'll be making some changes to the quip addition as part of bug 67950.

Comment 13

[Dave Miller [:justdave]]

moving to real milestones...

Comment 14

[Matthew Tuck [:CodeMachine]]

See also bug #73191.

Comment 15

[Gervase Markham [:gerv]]

  - adds a reset button to newquip.html

Reset buttons bad.

  - makes new_comment.cgi use CGI.pl
  - outputs the standard bugzilla footer

Done in bug 117759.

  - safely allows HTML in quips by using html_quote()

As discussed, the current solution is best.

  - checks whether adding the quip succeeded (check those return values!)

Can't remember if bug 117759 does this.

Gerv

Comment 16

[Dave Miller [:justdave]]

All the stuff covered here has either been done already as part of other bugs,
or we don't plan on doing (for the reasons stated in the above comments)

Comment 17

[Jacob Steenhagen]

V.