Don't echo back the provided path on a 404 error

[rkday]

I had a security scan flag up a potential XSS vulnerability on the fluentd Prometheus metrics port:

The request string used to detect this flaw was :
/cgi-bin/<script>cross_site_scripting.nasl</script>.asp
The output was :
HTTP/1.1 404 Not Found
Content-Type: text/plain
connection: close
content-length: 71
404 Not Found: /cgi-bin/<script>cross_site_scripting.nasl</script>.asp
Note that this XSS attack may only work against web browsers
that have "content sniffing" enabled.

This is because <script> tags in the URL are echoed back in plaintext.

I don't think this represents a real security vulnerability:

• you're unlikely to be interacting with fluentd in a browser
• the Content-Type is text/plain, so the browser would need to ignore it ("may only work against web browsers that have "content sniffing" enabled")
• even if the attack worked I think you'd only be able to steal information cached in the fluentd browser session, and I suspect it won't set cookies etc.

but I don't think echoing back the path adds much value, so I'd like to just remove it to avoid false positives.

I couldn't see a good way to HTML-escape this without pulling in a new dependency, and I didn't think that was worth doing.

I haven't opened an issue for this, just raised a PR - let me know if I should also raise an issue.

[ashie]

I couldn't see a good way to HTML-escape this without pulling in a new dependency, and I didn't think that was worth doing.

FYI: CGI.escapeHTML can be used.
It's included in standard library of Ruby so that you don't need to introduce a new dependency.
https://ruby-doc.org/stdlib-3.0.0/libdoc/cgi/rdoc/CGI/Util.html#method-i-escapeHTML

but I don't think echoing back the path adds much value, so I'd like to just remove it to avoid false positives.

I think so too.
The fix itself is O.K. for me but need to fix a failed test too.

2021-06-22T03:48:14.3720920Z Failure: test: use default app if path is not found(HttpHelperRouterTest::#mount)
2021-06-22T03:48:14.3723896Z /home/runner/work/fluentd/fluentd/test/plugin_helper/http_server/test_route.rb:23:in `block (2 levels) in <class:HttpHelperRouterTest>'
2021-06-22T03:48:14.3725730Z      20:       test 'use default app if path is not found' do
2021-06-22T03:48:14.3726977Z      21:         router = Fluent::PluginHelper::HttpServer::Router.new
2021-06-22T03:48:14.3728318Z      22:         req = flexmock('request', path: 'path/')
2021-06-22T03:48:14.3730261Z   => 23:         assert_equal(router.route!(:get, '/path/', req), [404, { 'Content-Type' => 'text/plain' }, "404 Not Found: #{req.path}\n"])
2021-06-22T03:48:14.3731397Z      24:       end
2021-06-22T03:48:14.3732036Z      25: 
2021-06-22T03:48:14.3733044Z      26:       test 'default app is configurable' do
2021-06-22T03:48:14.3734392Z <[404, {"Content-Type"=>"text/plain"}, "404 Not Found\n"]> expected but was
2021-06-22T03:48:14.3735808Z <[404, {"Content-Type"=>"text/plain"}, "404 Not Found: path/\n"]>
2021-06-22T03:48:14.3736577Z 
2021-06-22T03:48:14.3737209Z diff:
2021-06-22T03:48:14.3738272Z ? [404, {"Content-Type"=>"text/plain"}, "404 Not Found: path/\n"]

[ashie]

https://github.com/fluent/fluentd/blob/master/test/plugin_helper/http_server/test_route.rb#L23

[rkday]

Thanks - think I've fixed that test.

[ashie]

Thanks!