DOM based XSS in /dashboard

[hland]

It's possible to execute arbitrary Javascript in a user's browser by sending them a link such as: https://localhost/dashboard/#"><img src=x onerror=alert(1)>

This seems to be a a DOM based XSS that is reflected to the server when it tries to load the dashboard. It seems to be the error message that results when the dashboard is not found that is not filtering the input and escaping the output properly.

This code produces the error message: https://github.com/graphite-project/graphite-web/blob/master/webapp/graphite/dashboard/views.py#L258

And this code parses the error message: https://github.com/graphite-project/graphite-web/blob/master/webapp/content/js/dashboard.js#L2801

Finally, this line passes the unescaped error message to Ext.Msg.Alert:

var result = Ext.decode(response.responseText);
if (result.error) {
  Ext.Msg.alert("Error Loading Template", result.error);

Recommended fix:

• Do some filtering e.g. allow only something like[A-Za-z0-9-] for the dashboard name before sending a request to look up the dashboard.

• Do HTML output encoding (remember to encode according to the appropriate context if other instances are found)

[piotr1212]

Wouldn't it make sense to drop all the dasboarding features in favor of Grafana (and just keep the composer)? I guess there is a dozen more XSS bugs.

[DanCech]

The biggest thing missing in Grafana today is a tree-style explorer for finding metrics, but if that can be added then I think it makes sense.

[piotr1212]

Yes, I still use the "composer" tree based view. I really miss that in Grafana, it is really usefull if you are browsing metrics when you don't know what you are looking for. The dashboarding part always felt clumsy and buggy in Graphite.

[deniszh]

Yes, @piotr1212 says about http:/<GRAPHITE>/dashboard/ (https://github.com/graphite-project/graphite-web/tree/master/webapp/graphite/dashboard in sources), not about tree view, right? I also not used that, but I think we have a some users who still using that. Probably, some "dashboard-to-grafana" migration script could help.

[deniszh]

IIRC for example @cbowman0 using dashboards.

[cbowman0]

Yes. I think it would be wrong to remove the dashboard. We also use grafana but the use cases are different.

Of course I also patch back in graphlot support since that's used here, too.

[ytturi]

This vulnerability is still active. Even if you do not use the dashboard features, they are enabled by default in Graphite-web. If you have it as datasource in your grafana dashboards, then you can trace it to Graphite-web and perform malicious actions.

Alternative fix If you use an HTTP proxy, like nginx or apache, in front of your graphite-web then you can perform a regex check in the URI to disable the queries with certain characters, such as: "<" ">"

[deniszh]

Yes, this is need to be addressed.

[deniszh]

Closed in favor of https://github.com/graphite-project/graphite-web/issues/2520

[deniszh]

Ah, no, it's a different one.

[deniszh]

Fixed in #2785