412431 - Prevent UTF-7 tag injection in Gecko-generated directory listings (or other charsets)

Status: RESOLVED WORKSFORME

(Core :: Networking, enhancement)

Description

[Masahiro YAMADA]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; ja; rv:1.8.1.11) Gecko/20071127 Firefox/2.0.0.11
Build Identifier: 

Now, nsEscapeHTML2() in /mozilla/source/xpcom/io/nsEscape.cpp does not escapes "+".
But, It causes to enable UTF-7 encoded script injection for directory list.
See also Bug 411433.

Reproducible: Always

Steps to Reproduce:
(Testcase of UTF-7 tag injection to directory list)

1.Creare directory to some named
+ADw-/title+AD4APA-meta http-equiv+AD0-content-type content+AD0-text/html+ADs-charset+AD0-utf-7/+AD4-/
aa+ACI-+AD4-+ADw-script+AD4-alert(location)+ADw-+AC8-script+AD4-

# +ADw-/
# title+AD4APA-meta http-equiv+AD0-content-type content+AD0-text/
# html+ADs-charset+AD0-utf-7/
# +AD4-/
# aa+ACI-+AD4-+ADw-script+AD4-alert(location)+ADw-+AC8-script+AD4-

2.Show directory list
3.Change encoding of directory list as UTF-7
Actual Results:  
Script in directory name is executed

Expected Results:  
Script should not executed

If "+" is escaped as "+" , it does not causes UTF-7 encoded tag injection.

Comment 1

[Jesse Ruderman]

How about not letting users force different charset interpretations of internally-generated pages such as directory lists?  That sounds more foolproof to me than just working around the problem for *one* of the charsets with the "Byte sequences that don't contain '<' can be decoded as character sequences that contain '<'" property.

Comment 2

[Masahiro YAMADA]

(In reply to comment #1)
> How about not letting users force different charset interpretations of
> internally-generated pages such as directory lists?

Looks fine.
But, it is needed to fix bugs arond charset inheritance in subframe (Bug 356280, Bug 412420, etc.)

Comment 3

[Masahiro YAMADA]

If UI of directory list is NOT imlemented by <SCRIPT> in directory list (like Fx2), security risk of tag injection seems to be same as risk of showing file that has same path of direcotory.

# If JavaScript is disabled, both of "script in injected tag at directory list"
# and "script in file" will blocked. but it does not blocks UI of directory list.

But at firefox-trunk, to enable UI of directory, JavaScript MUST be enabled for directory list. And it also enables to execute injected JavaScript.
(See also Bug 411480).

I think, If we want to implement UI of directory list by JavaScript written in its html, We have to block tag injection strictly.

Comment 4

[Boris Zbarsky [:bzbarsky]]

I think Jesse is right.  Why not make this XHTML (instead of HTML) and then there is no charset injection issue of any kind?

Comment 5

[Jesse Ruderman]

UTF-7 was removed from browser menus in bug 441876, but I think there are other charsets with the same issue.

How does switching to XHTML help?  Does that disable all the charset menu items?

Comment 6

[Boris Zbarsky [:bzbarsky]]

Yes.  When doing XML, we only use the HTTP header and the XML declaration for charset information.  We do not use any other sources, including user overrides.  You can select whatever you want in the charset menu, and we'll reload the page, but whatever you selected in the menu will be ignored.

Comment 7

[Jesse Ruderman]

nsIndexedToHTML.cpp sets the charset of the entire page based on something from the server, so converting its output to XHTML might not be straightforward.  That was added in bug 162377.

Comment 8

[Masahiro YAMADA]

nsIndexedToHTML.cpp now uses HTML, not XHTML after bug 294800 fixed.

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=/mozilla/netwerk/streamconv/converters&command=DIFF_FRAMESET&file=nsIndexedToHTML.cpp&rev2=1.81&rev1=1.80

Comment 9

[Masahiro YAMADA]

I think, we can mark this bug as WONTFIX (or INVALID ) because UTF-7 support is already removed.

Comment 10

[Patrick McManus [:mcmanus]]

comment 9