Last Comment Bug 622981 - UTF-8 URLs ending with ? fail to load when a relative URL tries to change the query
: UTF-8 URLs ending with ? fail to load when a relative URL tries to change the...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Networking: Cache (show other bugs)
: unspecified
: All All
: -- major (vote)
: ---
Assigned To: [:jesup] on pto until 2016/7/5 Randell Jesup
:
Mentors:
http://vicerveza.homeunix.net/~viric/...
: 648626 656691 657033 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-01-04 14:06 PST by viriketo
Modified: 2011-08-10 21:34 PDT (History)
15 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
xpcshell testcase showing that URI resolution against a UTF-8-based based URI is wrong (1.09 KB, patch)
2011-02-22 15:50 PST, Brian Smith (:briansmith, :bsmith, use NEEDINFO?)
no flags Details | Diff | Review
xpcshell testcase showing that URI resolution against a UTF-8-based based URI is wrong (1.09 KB, text/plain; charset=UTF-8)
2011-02-22 15:59 PST, Brian Smith (:briansmith, :bsmith, use NEEDINFO?)
no flags Details
Patch to fix this bug and greatly expand the test coverage of test_URIs.js (26.42 KB, patch)
2011-07-09 19:23 PDT, [:jesup] on pto until 2016/7/5 Randell Jesup
bzbarsky: review+
Details | Diff | Review

Description viriketo 2011-01-04 14:06:36 PST
User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; ca-ES; rv:1.9.2.13) Gecko/20101220 Firefox/3.6.3
Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; ca-ES; rv:1.9.2.13) Gecko/20101220 Firefox/3.6.13

There appears to be a bad string manipulation regarding the URL, if the URL contains utf-8 characters, and a link in the webpage simply starts with a "?" (to only alter the query string, but not the URL path)

Reproducible: Always

Steps to Reproduce:


Let's say I have the browser looking at "http://localhost:8090/distribuïnt?"  (notice the unicode ï).

If the page has a link like: <a href="?local_images=0">[Use remote images]</a>
firefox renders the destination URL as: http://localhost:8090/distribu%C3%?local_images=0
When it should be: http://localhost:8090/distribu%C3%AFnt?local_images=0

It looks like the ?... is appended at the number of bytes determined by the number of utf-8 characters of the URL, instead of the number of bytes of the encoded URL.

Maybe this bad handling of strings can potentially end in an exploitable buffer overflow, additional to the inconvenience of failing "?blabla" links.
Actual Results:  
http://localhost:8090/distribu%C3%?local_images=0

Expected Results:  
http://localhost:8090/distribu%C3%AFnt?local_images=0
or showing directly:
http://localhost:8090/distribuïnt?local_images=0
Comment 1 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-01-13 12:58:42 PST
Would you be able to set up a test site as a proof of concept?
Comment 2 viriketo 2011-01-13 13:47:07 PST
Here you have a URL. The ? at the end seems to trigger the problem, as without it, all works.
http://vicerveza.homeunix.net/~viric/tmp/xènia?
Comment 3 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-02-18 14:37:41 PST
Going to http://vicerveza.homeunix.net/~viric/tmp/xènia and clicking TEST seems to work for me.

Going to http://vicerveza.homeunix.net/~viric/tmp/xènia? and clicking TEST gives me an Error 400 webserver error.

The same test under Safari results in no error.  I'm not sure what the issue is, but marking as NEW.
Comment 4 viriketo 2011-02-19 05:46:47 PST
After clicking the link to "xènia?" check the URL firefox goes to. The webserver error comes from the wrong URL firefox used for the link.
Comment 5 viriketo 2011-02-19 05:52:07 PST
Sorry, I meant that the case you should loook at is that giving 'webserver error' to you, due to the wrong url. As you changed the subject of the bug report, I imagine you understood all well.
Comment 6 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2011-02-22 15:50:49 PST
Created attachment 514344 [details] [diff] [review]
xpcshell testcase showing that URI resolution against a UTF-8-based based URI is wrong

Navigating the link via clicking (e.g. by clicking on the link in this bug's "URL" field) and then clicking the "Test" link on the resultant page works correctly. But, copy+pasting the URL and then clicking the "Test" link on the resultant page gives the described erroneous behavior. The behavior is the same in 3.5.16 so this is not a regression and thus shouldn't be a blocker.

The attached xpcshell testcase demonstrates the behavior.
Comment 7 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2011-02-22 15:59:58 PST
Created attachment 514346 [details]
xpcshell testcase showing that URI resolution against a UTF-8-based based URI is wrong
Comment 8 [:jesup] on pto until 2016/7/5 Randell Jesup 2011-07-08 23:47:45 PDT
Taking bug
Comment 9 [:jesup] on pto until 2016/7/5 Randell Jesup 2011-07-09 14:47:18 PDT
The cause is ::ParsePath() finding the query in "native" form (i.e. with the characters as single bytes), and then the actual spec being stored in encoded (%XX) form.  This causes the offsets for the query (and probably param or ref) to be wrong.  I believe there's no chance of buffer overflow, since it will always indicate a position and length (when it's wrong) that's too early/short, not too late/long.

Looking at the best way to reorder things to fix this.
Comment 10 [:jesup] on pto until 2016/7/5 Randell Jesup 2011-07-09 19:23:32 PDT
Created attachment 545042 [details] [diff] [review]
Patch to fix this bug and greatly expand the test coverage of test_URIs.js

The test coverage is largely based on the RFC 3986 examples.  Some tests don't work easily in the testing framework and have been disabled for now.  The tests should hit all the major cases and many edge cases.

One very minor bug was caught by this; the test for that is disabled and I'll submit something for that later.

The bug 622981 test (two simple relative URLs) could be spun into a separate bug file instead of mixing it with all the basic functionality tests.
Comment 11 [:jesup] on pto until 2016/7/5 Randell Jesup 2011-07-09 19:33:48 PDT
*** Bug 648626 has been marked as a duplicate of this bug. ***
Comment 12 [:jesup] on pto until 2016/7/5 Randell Jesup 2011-07-09 19:46:00 PDT
Make the title reflect the actual bug
Comment 13 [:jesup] on pto until 2016/7/5 Randell Jesup 2011-07-09 19:53:01 PDT
*** Bug 657033 has been marked as a duplicate of this bug. ***
Comment 14 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-07-12 10:23:07 PDT
Comment on attachment 545042 [details] [diff] [review]
Patch to fix this bug and greatly expand the test coverage of test_URIs.js

r=me.  I really wish the invariants for this code were better documented.... :(

Thanks for the patch!

If you want someone else to push this, please don't forget to post a patch with a From or User line and a commit message.
Comment 15 [:jesup] on pto until 2016/7/5 Randell Jesup 2011-07-12 11:35:54 PDT
Checked in: http://hg.mozilla.org/mozilla-central/rev/9fe4289527fc
Comment 16 [:jesup] on pto until 2016/7/5 Randell Jesup 2011-07-17 22:05:04 PDT
*** Bug 656691 has been marked as a duplicate of this bug. ***
Comment 17 [:jesup] on pto until 2016/7/5 Randell Jesup 2011-08-10 21:34:39 PDT
*** Bug 677851 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.