Possible XSS via ISO-2022-JP encoding

RESOLVED FIXED in mozilla56

Status

()

RESOLVED FIXED
3 years ago
6 months ago

People

(Reporter: websec02.g02, Assigned: hsivonen)

Tracking

({sec-moderate})

42 Branch
mozilla56
sec-moderate
Points:
---

Firefox Tracking Flags

(firefox-esr52 wontfix)

Details

(Whiteboard: [fixed by encoding_rs])

Attachments

(1 attachment, 1 obsolete attachment)

108 bytes, text/html;charset=iso-2022-jp
Details
(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:42.0) Gecko/20100101 Firefox/42.0
Build ID: 20151029151421

Steps to reproduce:

Serves a web page in ISO-2022-JP charset contains the following byte sequence.

<p>[0x1B][0x1B]$B<SCRIPT>eval("al\ert\(document\.domain\)")</SCRIPT>X[0x1B](B</p>

"[0x1B]$B" in the bytes above is an escape sequence of JIS X 0208 in ISO-2022-JP, and "[0x1B](B" is an escape sequence of ASCII.



Actual results:

The bytes from "[0x1B]$B" to "[0x1B](B" should be treated as JIS X 0208, not ASCII. Major browsers other than Firefox treat it as such, but firefox interprets it as an ASCII data and execute the JavaScript (it shows a popup window.)

This happens in Firefox, because, in the HTML above, ESC ([0x1B]) is placed immediately before the escape sequence "[0x1B]$B". Firefox seems to ignore a byte occurs right after a bare ESC char that does not constitute a valid escape sequence.

Note that Some ISO-2022-JP encoders, including one used in Firefox, seem to produce a string containing a bare ESC when encoding. Thus, maybe decoders should assume a bare ESC occurence while decoding.

P.S.
The severity of the bug may not be high, as ISO-2022-JP (as web page encoding) isn't common.

Comment 1

3 years ago
Henri, I remember you did some work with our charset code. Are you the right person to look at this or do you want to nominate someone else?
Group: firefox-core-security → core-security
Component: Untriaged → Untriaged
Flags: needinfo?(hsivonen)
Product: Firefox → Core
(Assignee)

Comment 2

3 years ago
Posted file Test case (obsolete) —
Flags: needinfo?(hsivonen)
(Assignee)

Updated

3 years ago
Attachment #8687555 - Attachment is patch: false
Attachment #8687555 - Attachment mime type: text/plain → text/html
(Assignee)

Comment 3

3 years ago
Posted file Test case, for real
Attachment #8687555 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 4

3 years ago
I was hoping we could rewrite the ISO-2022-JP decoder in Rust some time in the future in a relaxed way. Now it looks like we need to do a partial rewrite as a security bug. :-(

Basically all the 'else' branches in the escape states are bogus and need to be rewritten to decrement src, then readjust mState and then either signal error or emit U+FFFD. Additionally, for the decrement to work, we need to read src to a variable and increment src at the top of the loop. Instead of reading *src all over and incrementing at the end, since it's not done if the output buffer is found to be full first.
Component: Untriaged → Internationalization
(Assignee)

Comment 5

3 years ago
(In reply to Henri Sivonen (:hsivonen) from comment #4)
> Basically all the 'else' branches in the escape states are bogus and need to
> be rewritten to decrement src, then readjust mState and then either signal
> error or emit U+FFFD. Additionally, for the decrement to work, we need to
> read src to a variable and increment src at the top of the loop. Instead of
> reading *src all over and incrementing at the end, since it's not done if
> the output buffer is found to be full first.

Looks like a middle-ground fix like that won't work. Either we need to rewrite the loop and states according to the Encoding Standard or we need to add checks for 0x1B to each state and make transitions to mState_ESC. If we do the former, we might as well get rid of the ISO-2022-JP-2 stuff while at it. If the we do the latter, getting the error reporting right is going to be annoying.
(Assignee)

Comment 6

3 years ago
emk, smontagu, are you OK with removing the ISO-2022-JP-2 features in a security fix?
Flags: needinfo?(smontagu)
Flags: needinfo?(VYV03354)
(Assignee)

Comment 7

3 years ago
(In reply to :Gijs Kruitbosch from comment #1)
> Henri, I remember you did some work with our charset code. Are you the right
> person to look at this or do you want to nominate someone else?

I probably won't have the time to *fix* this in the coming days, so if emk or smontagu wants to take the bug, please go ahead.
Let's file a non-security bug and send "Intent to unship" to dev-platform for the removal of ISO-2022-JP-2 support.
Flags: needinfo?(VYV03354)
See also bug 715833 comment 2 which implies that we still need iso-2022-jp-2 for email. That was three years ago -- if Masatoshi is happy removing it today, I have no objection.
Flags: needinfo?(smontagu)

Comment 10

3 years ago
Fixing bug 715833 seems like the way to go.

Note also https://github.com/whatwg/encoding/issues/15 about the iso-2022-jp encoder which I have not addressed yet.
Group: core-security → layout-core-security
Triage committee is trying to figure out a security severity rating for this. What are the implications of this problem?

Comment 12

3 years ago
Well, when a site uses iso-2022-jp (unlikely) and accepts user input, it may be susceptible to XSS if a) they accept user input and output that somewhere on their site and b) their server iso-2022-jp decoder/encoder pair is different from the one in Firefox (likely).
(Assignee)

Comment 13

3 years ago
(In reply to Anne (:annevk) from comment #12)
> Well, when a site uses iso-2022-jp (unlikely) and accepts user input, it may
> be susceptible to XSS if a) they accept user input and output that somewhere
> on their site and b) their server iso-2022-jp decoder/encoder pair is
> different from the one in Firefox (likely).

More precisely, it seems to me this requires the site not to decode and re-encode.

So:

If all the following hold

 1) Site use ISO-2022-JP
 2) Site accepts user input
 3) Site serves input from one user to another user
 4) Site serves the bytes the bytes that it received from a user (as opposed to performing some processing that converts from ISO-2022-JP into something else and then back to ISO-2022-JP resulting in freshly-encoded bytes)

...the site may be vulnerable to one user being able to inject scripts that run on the origin of the site in another user's Firefox.

The outcome qualifies as sec-high, but the preconditions limit exploitability. For starters, only 0.05% of release channel sessions instantiate the ISO-2022-JP decoder at least once during the session.
(Assignee)

Comment 15

3 years ago
(to unship the -2 parts, that is.)
Keywords: sec-moderate
(Assignee)

Updated

3 years ago
Depends on: 1261841
(Assignee)

Updated

2 years ago
Attachment #8687557 - Attachment mime type: text/html → text/html;charset=iso-2022-jp
(Assignee)

Comment 16

2 years ago
Fixed by bug 1261841. (The test case used to have the wrong HTTP charset. I verified after changing the attachment properties moments ago.)
Assignee: nobody → hsivonen
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Whiteboard: [fixed by encoding_rs]
Target Milestone: --- → mozilla56
Group: layout-core-security → core-security-release
I assume we won't implement the rewrite in bug 1261841 on the 52ESR branch.
status-firefox-esr52: --- → wontfix
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.