Security Vulnerability in Bar Chart Tooltip

[calvinkcox]

The default tooltip implementation is not encoding HTML correctly. This could cause a security issue where cross site scripting could be used to gather a user's data without their knowledge.

Using the bar chart:
http://c3js.org/samples/chart_bar.html

With data:
var chart = c3.generate({
data: {
columns: [
['data1">', 30, 200, 100, 400, 150, 250],
['data2', 130, 100, 140, 200, 150, 50]
],
type: 'bar'
},
bar: {
width: {
ratio: 0.5 // this makes bar width 50% of length between ticks
}
// or
//width: 100 // this makes bar width 100px
}
});

[aendra-rininsland]

@calvinkcox Oh wow, sorry we haven't gotten to this sooner!

[calvinkcox]

@Aendrew To really be an issue someone would have to be pulling in a 3rd party data source that is/becomes malicious. It's certainly a problem, but I doubt it's affecting anyone at the moment.

[aendra-rininsland]

@calvinkcox Yep, am not too worried it's causing anything in the wild, but there still should be better data sanitization than that. 😄

[masayuki0812]

I added sanitise to avoid raw html in tooltip. I think now it's ok.

[kavacky]

It was not a security vulnerability. It should be developer's responsibility to sanitise output when necessary. Forcing all tooltip formatting to be sanitised afterwards simply breaks formatting capabilities for those who want use some HTML and CSS to do that.

Why can't I use my perfectly safe HTML tags in tooltip format anymore? At least there could have been an option to skip this "very useful" saniTisation part. Why did you fuck this up?

Now I have to hack into c3.js and use a modified version to skip this stupid shit to be able to do again what simply worked out of the box before.

[calvinkcox]

@kavacky You could certainly fork the code and put up a pull request to add an optional param allow HTML (see: https://docs.angularjs.org/api/ngSanitize/service/$sanitize). The HTML tags were certainly not my original concern, the JS parsing was.

If you look at other libraries (Angular even) HTML and JS is not interpolated by default and has to be allowed via a flag/special attribute (https://docs.angularjs.org/api/ng/directive/ngBindHtml).

[martingraham]

Bit late now, but to reverse this you don't need to hack the actual c3 code, just add this to your tooltip config:

                contents: function (d, defaultTitleFormat, defaultValueFormat, color) {
                    var text = this.getTooltipContent (d, defaultTitleFormat, defaultValueFormat, color);
                    return text.replace(/&lt;/g, '<').replace(/&gt;/g, '>');
                },

That's on the basis you know full well what data and series names you're displaying, i.e. doesn't contain hacky strings like onload="sendmeyourcreditcarddetails", my data is just numbers and I want to align them around the decimal point so I do a bit of styling, and my series names are hardcoded

Also caveat is that if you also have unmatched > < 's that were in there before the sanitisation (i.e. as part of series names or the data), it'll fart and die so be careful

You could also just not use the default getTooltipContent function and build your own tooltip from scratch in the contents: declaration