Bug #2251

URI.parse accepts strings with invalid characters

Added by emerose (Sam Quigley) over 2 years ago. Updated about 1 year ago.

[ruby-core:26223]
Status:Closed Start date:10/22/2009
Priority:Normal Due date:
Assignee:akira (akira yamada) % Done:

100%
Category:-
Target version:-
ruby -v:ruby 1.9.1p243 (2009-07-16 revision 24175) [i386-darwin10.0.0]

Description

The regexes used in URI::Parser's initialize_regexp use ^ and $ rather than \A and \Z:

   399       # for URI::split
   400       ret[:ABS_URI] = Regexp.new('^' + pattern[:X_ABS_URI] + '$', Regexp::EXTENDED)
   401       ret[:REL_URI] = Regexp.new('^' + pattern[:X_REL_URI] + '$', Regexp::EXTENDED)

The result is that URI.parse matches on any URI separated by newlines, rather than on its argument as a whole:

   irb(main):001:0> require 'uri'
   => true
   irb(main):002:0> URI.parse("blah\nhttp://www.foo.com/\nblahblah")
   => #<URI::HTTP:0x000001010aac78 URL:http://www.foo.com/>

I think programmers would expect URI.parse to only successfully parse strings that *are* URIs, rather than any string that *contains* a URI surrounded by a particular kind of whitespace.  This issue has apparently caused at least one security vulnerability in the real world: http://schmoil.blogspot.com/2009/10/mainlining-new-lines-feel-burn.html

Replacing the ^ and $ with \A and \Z should fix the issue, and is unlikely to break any existing code.  The Rubyspec project does not seem to have any tests for this behavior.  This behavior is present in at least versions 1.8.6, 1.8.7, and 1.9.1.

-sq

Associated revisions

Revision 26229
Added by naruse (Yui NARUSE) over 2 years ago

* lib/uri/common.rb (initialize_regexp): allow leading and trailing white space, and forbid extra characters on another lines. [ruby-core:26223] RFC 3986 Appendix C. Delimiting a URI in Context draft-duerst-iri-bis-07 7.2. Web Address processing

History

#1
Updated by naruse (Yui NARUSE) over 2 years ago

  • Status changed from Open to Assigned
  • Assignee set to akira (akira yamada)

#2
Updated by naruse (Yui NARUSE) over 2 years ago

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100
This issue was solved with changeset r26229.
Sam, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

Also available in: Atom PDF