Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dealing with invalid host names #54

Closed
assaf opened this issue Feb 6, 2015 · 5 comments
Closed

Dealing with invalid host names #54

assaf opened this issue Feb 6, 2015 · 5 comments
Labels

Comments

@assaf
Copy link

assaf commented Feb 6, 2015

Someone sniffing for open vulnerabilities send our server a request with the URL: http://('4drsteve.com', [], ['54.213.246.177'])/xmlrpc.php

Our server handles traffic for multiple domains, so it passes the Host header toTLD.js to determine which domain the request belongs to. TLD.js extracts the tail portion of the value (177'])), which has no matching rule, and attempts to create a rule by turning this string into a regular expression without escaping it first.

So we get:

SyntaxError: Invalid regular expression: /.+\.177'])$/: Unmatched ')' 
File "/app/node_modules/tldjs/lib/tld.js", line 107, in <unknown>
        if ((new RegExp(pattern)).test(host)) {
  File "/app/node_modules/tldjs/lib/tld.js", line 41, in _someFunction
          if (i in t && fun.call(thisArg, t[i], i, t))
  File "/app/node_modules/tldjs/lib/tld.js", line 91, in Function.getCandidateRule
      _someFunction(rules, function (r) {
  File "/app/node_modules/tldjs/lib/tld.js", line 247, in tld.getDomain
      rule = tld.getCandidateRule(host, rules);

Not sure where to patch fix, probably in Rule constructor when creating a new rule it should always attempt to escape a string?

Or should this even be patched? Should TLD.js throw an error on invalid host names, or just return null?

@thom4parisot
Copy link
Owner

This is not a valid hostname so I guess it could be addressed by #53.

Thanks for reporting it :-)

myndzi pushed a commit to myndzi/tld.js that referenced this issue May 25, 2015
@myndzi myndzi mentioned this issue May 25, 2015
@thom4parisot
Copy link
Owner

Let us know if the 1.5.3 fixed your issue. If so, you can thank @myndzi and close the issue :-)

@assaf
Copy link
Author

assaf commented Jun 24, 2015

Still getting the same error with 1.5.3:

> tld.getDomain('(\'4drsteve.com\', [], [\'54.213.246.177\'])')
SyntaxError: Invalid regular expression: /.+\.177'])$/: Unmatched ')'

@myndzi
Copy link
Collaborator

myndzi commented Jun 25, 2015

Herpaderp. It works when there's a path but not just a hostname. Kind of a problem with accepting mixed format inputs I guess. I'll check it out a little further. My main instinct is to expand cleanHostValue to return only valid hostname characters, which are all acceptable as regular expression characters. Not entirely sure why a regex is being composed from the input, but I don't think I have to internalize all that to fix this problem.

@thom4parisot
Copy link
Owner

@assaf closing as @myndzi fixed it in #60

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants