Skip to content

Comments

404 messages shouldn't display the requested path without escaping#1070

Closed
cdmalon wants to merge 5 commits intoPerlDancer:masterfrom
cdmalon:bugfix/injection_in_404
Closed

404 messages shouldn't display the requested path without escaping#1070
cdmalon wants to merge 5 commits intoPerlDancer:masterfrom
cdmalon:bugfix/injection_in_404

Conversation

@cdmalon
Copy link
Contributor

@cdmalon cdmalon commented Dec 10, 2015

Currently, a request for a nonexistent path results in a 404 error that displays the requested path, without escaping.

So, if you request

http://myserver/nonexistent_path<strong>crazy</strong>

you will see "crazy" in bold. This opens up HTML injection possibilities. This pull request simply removes the requested path from the information sent to the 404 template.

cdmalon added 5 commits March 26, 2015 19:52
Dancer2::Core::Error::throw() attempts to set the response using set_response(), but response is 'rw', not 'rwp'.
Error.pm - fix broken accessor
404 shouldn't display the requested path as part of the response HTML without escaping.
@SysPete
Copy link
Member

SysPete commented Dec 10, 2015

My view is that including untrusted data in a template without escaping it is a failure of the template / template engine. I personally use a template engine which escapes by default so this issue does not affect me but this patch does since I use the message token in my 404.

Time for me to look further...

@SysPete
Copy link
Member

SysPete commented Dec 11, 2015

So this only happens when show_errors is set to true which should never be used in a production environment. My gut feeling is that we should _html_encode($self->message) in Dancer2::Core::Error::default_error_page anyway.

Thoughts anyone?

@veryrusty
Copy link
Member

@SysPete I agree with your gut feel.

@cromedome
Copy link
Contributor

@SysPete agreed.

@SysPete
Copy link
Member

SysPete commented Dec 11, 2015

SInce I have already submitted PR #1071 to rectify as per my earlier comment I'm going to close this PR.

@cdmalon many thanks for catching this issue.

@SysPete SysPete closed this Dec 11, 2015
xsawyerx added a commit that referenced this pull request Dec 16, 2015
    [ DOCUMENTATION ]
    * Update core team members and contributors list. (Russell Jenkins)
    * GH #1066: Fix typo in Cookbook. (gertvanoss)
    * Correct typo. It's "query_parameters", not "request_parameters".
      Thanks to mst for letting me know and making sure I fix it!
      (Sawyer X)

    [ BUG FIXES ]
    * GH #1040: Forward with a post body no longer tries to re-read body
      filehandle. (Bas Bloemsaat)
    * GH #1042: Add Diggest::SHA as explicit prequisite for installs on
      perl < v5.9.3. (Russell Jenkins)
    * GH #1071, #1070: HTML escape the message in the default error page.
      (Peter Mottram)
    * GH #1062, #1063: Command line interface didn't support
      "-s SKELETON_DIRECTORY" in any order.
      (Nuno Carvalho)
    * GH #1052, #1053: Always call before_serializer hook when serializer
      is set.
      (Mickey Nasriachi)
    * GH #1034: Correctly use different session cookie name for Dancer2.
      (Jason A. Crome)
    * GH #1060: Remove trailing slashes when providing skeleton
      directory.
      (Gabor Szabo)

    [ ENHANCEMENTS ]
    * Use Plack 1.0035 to make sure you only have HTTP::Headers::Fast
      in the Plack::Request object internally.
    * GH #951 #1037: Dancer2::Template::TemplateToolkit no longer sets TT2
      INCLUDE_PATH directive, allowing `views` setting to be non-absolute
      paths. (Russell Jenkins)
    * GH #1032 #1043: Add .dancer file to new app scaffolding.
      (Jason A. Crome)
    * GH #1045: Small cleanups to Request class. (Russell Jenkins)
    * GH #1033: strict && warnings in Dancer2::CLI. (Mohammad S Anwar)
    * GH #1052, #1053: Allow before_serializer hook to change the content
      using @_.
      (Mickey Nasriachi)
    * GH #1060: Ignore .git directory when using an external skeleton
      directory.
      (Gabor Szabo)
    * GH #1060: Support more asset file extensions. (Gabor Szabo)
    * GH #1072: Add request->is_options(). (Theo van Hoesel)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants