Skip to content

XSS in examples #3992

Description

@jthorpe6

Hello,

just been playing around with the examples and i discovered XSS in the route-map example.

starting the app like so

~/express$ node examples/route-map/
get /users
delete /users
get /users/:uid
get /users/:uid/pets
delete /users/:uid/pets/:pid
Express started on port 3000

Then browsing to the following causes the injected javascript to load
http://ip:3000/users/%22%3E%3Csvg%20onload=prompt()%3E

full request

GET /users/%22%3E%3Csvg%20onload=prompt()%3E HTTP/1.1
Host: 192.168.122.246:3000
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Firefox/60.0
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate
Connection: close
Upgrade-Insecure-Requests: 1

the issue seems to be with the following in the codebase

var users = {
  list: function(req, res){
    res.send('user list');
  },
 
  get: function(req, res){
    res.send('user ' + req.params.uid);
  },
 
  delete: function(req, res){
    res.send('delete users');
  }
};

where req.params.uid is not sanitised

thanks

Activity

  1. wesleytodd commented on Jun 25, 2019

    @wesleytodd
    Member

    While I am not too worried because the examples are just to show express features, not real use, I think it would be good to do one of two things here:

    1. Add a comment about why rendering user content without escaping is bad
    2. Escape the input

    I would approve a PR which did either of the above.

  2. Caprico1 commented on Jun 25, 2019

    @Caprico1
  3. Caprico1 commented on Jun 25, 2019

    @Caprico1

    If it's within the examples odds are this is out in production somewhere. As a developer would look to examples either first starting out with the framework or just day to day. I can speak from experience on developers using example routes to build out there applications. Therefore since there is the possibility of it existing at least once within the base code, it's not a stretch for it to have been implemented into someone's production application.

  4. wesleytodd commented on Jun 25, 2019

    @wesleytodd
    Member

    By all means, fix it and submit a PR :)

  5. dougwilson commented on Jun 25, 2019

    @dougwilson
    Contributor

    Hi @Caprico1 and no one is claiming otherwise. But of course fixing the example won't somehow fixes those theoretical production deployments, for example. That's also why the suggestion was as part of a pull request to fix the issue, to add information for the users who are looking at the examples to understand escaping and why the example is doing it, etc.

  6. Caprico1 commented on Jun 25, 2019

    @Caprico1

    Right, it was to explain in an issue why this would be bad if in prod (owasp link)

    and @jthorpe6 and I's thought process on how this is an issue.

    I'll look into making a solution or at the least adding an example to sanitize inputs within routes within the framework.

  7. dataf3l commented on Jul 23, 2019

    @dataf3l

    Adding sanitization may not fix the existing codebases problem but it will address future users.

  8. krzysdz commented on Sep 13, 2019

    @krzysdz
    Contributor

    Speaking of security issues in the examples, it is possible to read any file (directory traversal) on a server running downloads example.

    %2E%2E%2F is decoded as ../ and causes path.join() to go to a parent directory.

    app.get('/files/:file(*)', function(req, res, next){
    var filePath = path.join(__dirname, 'files', req.params.file);

    And for example, it could be used to read a session secret:
    http://localhost:3000/files/%2E%2E%2F%2E%2E%2Fsession%2Fredis.js

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions