Closed Bug 963322 Opened 6 years ago Closed 6 years ago

[B2G][Browser] Multiple "Unable to connect" errors in browser app have %20 in place of spaces

Categories

(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect, P1, blocker)

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:1.3+, b2g-v1.3 verified)

RESOLVED FIXED
1.3 C3/1.4 S3(31jan)
blocking-b2g 1.3+
Tracking Status
b2g-v1.3 --- verified

People

(Reporter: bzumwalt, Assigned: aus)

References

Details

(Whiteboard: dogfood1.3 [systemsfe][p=1])

Attachments

(4 files)

Attached image Screenshot - 1
Description:
Multiple "Unable to connect" errors in the browser app appear to have parsing issues where "%20" appears in place of character spaces.

Errors include:

"The%20URL%20is%20not%20valid%20and%20cannot%20be%20loaded." ("The URL is not valid and cannot be loaded." - Received on trying to navigate to http://:5000)

"This%20address%20uses%20a%20network%20port%20which%20is%20normally%20used%20for%20purposes%20other%20than%20Web%20browsing.%20Firefox%20has%20cancelled%20the%20request%20for%20your%20protection" ("This address uses a network port which is normally used for purposes other than Web browsing. Firefox has cancelled the request for your protection" - Received on trying to navigate to http://::1/)

Repro Steps:
1) Updated Buri to BuildID: 20140121004137
2) Open browser app
3) Type "::1" into URL bar and tap enter

Actual:
Incorrect string parsing causes error text to be difficult to read.

Expected:
All error messages easily encountered by end user are readily readable.

Environmental Variables:
Device: Buri v1.3 Mozilla RIL
BuildID: 20140121004137
Gaia: 47049555282a9a01fb60d1e1421b57e2810c96f5
Gecko: 6f7dfe36ab6c
Version: 28.0a2
Firmware Version: V1.2-device.cfg

Notes:

Repro frequency: 3/3, 100%
See attached: screenshot
Attached image Screenshot - 2
Those characters shouldn't even be possible to get into that error message. Is this vulnerable to XSS?

Paul - Do you know?
Blocks: 882186
Group: b2g-core-security
Component: Gaia::Browser → Gaia::System
Flags: needinfo?(ptheriault)
(In reply to Jason Smith [:jsmith] from comment #2)
> Those characters shouldn't even be possible to get into that error message.
> Is this vulnerable to XSS?
> 
> Paul - Do you know?

This btw is referring to the app error page itself, as I'm surprised that %20 could be rendered here in a spot of the app error page doesn't support custom input.
Component: Gaia::System → Gaia::System::Window Mgmt
This is not an XSS vulnerability. That message string is coming from gecko, but even if someone could hijack that message, we use .textContent to render the message:

https://github.com/mozilla-b2g/gaia/blob/4347460dfda2c6a75c39164e389293bb73591122/apps/system/js/net_error.js#L128

However, I think this is still a bug. We should be decoding those characters before setting them as textContent.
Group: b2g-core-security
Flags: needinfo?(ptheriault)
Whiteboard: dogfood1.3 → dogfood1.3 [systemsfe][p=1]
When this happens, this is bad, as it makes reading the error page incredibly difficult to understand. However, I'm not sure of what the probability is of reproducing this.
(In reply to Jason Smith [:jsmith] from comment #5)
> When this happens, this is bad, as it makes reading the error page
> incredibly difficult to understand. However, I'm not sure of what the
> probability is of reproducing this.

Probability meaning the likelihood an end-user would actually run into this bug.
(In reply to Jason Smith [:jsmith] from comment #6)
> (In reply to Jason Smith [:jsmith] from comment #5)
> > When this happens, this is bad, as it makes reading the error page
> > incredibly difficult to understand. However, I'm not sure of what the
> > probability is of reproducing this.
> 
> Probability meaning the likelihood an end-user would actually run into this
> bug.

Nailed down the probability - this reproduces with any app error that deals with unknown protocol error. That's realistic for an end-user to hit on the web, so this is worth nominating.
blocking-b2g: --- → 1.3?
(In reply to Jason Smith [:jsmith] from comment #7)
> (In reply to Jason Smith [:jsmith] from comment #6)
> > (In reply to Jason Smith [:jsmith] from comment #5)
> > > When this happens, this is bad, as it makes reading the error page
> > > incredibly difficult to understand. However, I'm not sure of what the
> > > probability is of reproducing this.
> > 
> > Probability meaning the likelihood an end-user would actually run into this
> > bug.
> 
> Nailed down the probability - this reproduces with any app error that deals
> with unknown protocol error. That's realistic for an end-user to hit on the
> web, so this is worth nominating.

This looks like a regression too - I'm actually hitting this with all fallback errors. I don't recall hitting this previously, so I think this is a regression.
At a minimum, I know this worked when bug 952305 originally landed (which was on 1/9/2014).
Also - this is being seen on the iframe app error UX as well. So the regression here might be in gecko.
The regression window is 1/9/2014 to 1/10/2014.  Also on 1/9/2014 and earlier "no network" error message is given instead of the actual results in Comment 0.

First build issue reproduces on:
v1.3 Environmental Variables:
Device: buri v1.3 MOZ
BuildID: 20140110004009
Gaia: c606b129a2d1647c0fc7bfb197555026e9b27f9e
Gecko: c5109884ae3a
Version: 28.0a2
Firmware Version: v1.2-device.cfg

Last build issue did not reproduce on:
v1.3 Environmental Variables:
Device: buri v1.3 MOZ
BuildID: 20140109004002
Gaia: 22bc6be5b76cdc6d4e9667ff070979041a20ce2f
Gecko: 2c8f8683bd0d
Version: 28.0a2
Firmware Version: v1.2-device.cfg
QA Contact: pbylenga
(In reply to Peter Bylenga from comment #11)
> The regression window is 1/9/2014 to 1/10/2014.  Also on 1/9/2014 and
> earlier "no network" error message is given instead of the actual results in
> Comment 0.
> 
> First build issue reproduces on:
> v1.3 Environmental Variables:
> Device: buri v1.3 MOZ
> BuildID: 20140110004009
> Gaia: c606b129a2d1647c0fc7bfb197555026e9b27f9e
> Gecko: c5109884ae3a
> Version: 28.0a2
> Firmware Version: v1.2-device.cfg
> 
> Last build issue did not reproduce on:
> v1.3 Environmental Variables:
> Device: buri v1.3 MOZ
> BuildID: 20140109004002
> Gaia: 22bc6be5b76cdc6d4e9667ff070979041a20ce2f
> Gecko: 2c8f8683bd0d
> Version: 28.0a2
> Firmware Version: v1.2-device.cfg

Guess that implies this isn't a regression. Maybe this patch just worked locally, but failed upon checkin?
Keywords: regression
blocking-b2g: 1.3? → 1.3+
I'll have a look at this today. Should be an easy fix.
Assignee: nobody → aus
Status: NEW → ASSIGNED
Attachment #8366957 - Flags: review?(mhenretty)
Same changes, but for 1.3t
Attachment #8366958 - Flags: review?(mhenretty)
Severity: normal → blocker
Priority: -- → P1
Target Milestone: --- → 1.3 C3/1.4 S3(31jan)
Comment on attachment 8366957 [details] [review]
Pull request - Mainline 1.3

Code looks good. Verified on 1.3 Keon that this patch fixes the bug. Let's wait for travis to go green, then merge (I restarted the build since it looks like NPM was down again yesterday).
Attachment #8366957 - Flags: review?(mhenretty) → review+
Comment on attachment 8366958 [details] [review]
Pull Request - Tarako 1.3

Same as above, although, I don't have an actual device to test the code.
Attachment #8366958 - Flags: review?(mhenretty) → review+
Fixed on 1.3:  Commit = https://github.com/mozilla-b2g/gaia/commit/8defa5bf0cbce290c649e564b7f3ebe708e19b23
Fixed on 1.3t: Commit = https://github.com/mozilla-b2g/gaia/commit/7c68649158ec610665ff5e6b75e7ec5d483a91ce

Marking fixed.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Keywords: verifyme
QA Contact: pbylenga → jsmith
lgtm on 1.3 with some sanity checking with content errors in browser & app.

Why wasn't this landed on master?
Flags: needinfo?(aus)
Keywords: verifyme
(In reply to Jason Smith [:jsmith] from comment #19)
> lgtm on 1.3 with some sanity checking with content errors in browser & app.
> 
> Why wasn't this landed on master?

I can answer that. Between aurora and m-c, there was a change in gecko that did the decoding for us (that's why the screenshot in original bug was correct). Sadly, this change never made it into 1.3's gecko.
Flags: needinfo?(aus)
You need to log in before you can comment on or make changes to this bug.