content attribute for timeline item started striping HTML code breaking timeline renders

[jamiegau]

This morning, I logged into a few internal tools I wrote that use vis-timeline, and utilise the
https://unpkg.com/vis-timeline@latest/standalone/umd/vis-timeline-graph2d.min.js
to import the package.
Two different apps started displaying the timeline wrong. Meaning it must be pulling in a newer version that is now broken.
Broken in the sense that I set the "content" attribute of a timeline item with a lot of HTML code to render quite a bit of data in the timeline items. The content appears to be "STRIPPED" of certain html code now resulting in a broken render of the timeline item.

I have been digging around and have not found any reference to this behaviour or change.
Can someone help me out here, is there a way around this new bug, or maybe it's an intentional sanitization of the content? No idea. but there must be a way to fix this.

I would consider this a major issue that needs to be addressed quickly as any tool leveraging the public unpkg version above is likely now broken. (Like my tools).

Thanks
James

[jamiegau]

Ok, I see that this security fix #840 appears the be the culprit.

[jamiegau]

Note, if you're hitting this issue. Pin to the release before the security fix. (v7.4.3)

<script type="text/javascript" src="https://unpkg.com/vis-timeline@7.4.3/standalone/umd/vis-timeline-graph2d.min.js"></script>

[JohnJVTK]

Hello !
I have the same issue as you since I updated to 7.4.4 and also in the (very) recently 7.4.5.
I'll stay in 7.4.3 while waiting for a solution.
Thanks

[Thomaash]

Hi there,

you should also be able to bypass the xss module via an element, regardless of what version you're using:

const element = document.createElement("script");
element.innerHTML = 'alert("You\'ve been hacked, muheheheheheee :-).");';

items.add({
  ...normalItemStuff,
  title: element,
});

[JohnJVTK]

Thanks for your reply.
Creating element this way solved my problem.

In my .Net Core 5.0.1 app I have this method

        [HttpPost]
        public JsonResult GetDataTimeline([FromBody] DataTimelineViewModel viewModel)
        {
            var data = _demandeCongeService.GetDataTimeline(viewModel.Users, GetUserId());

            var items = data
                .Select(x => new
                {
                    start = x.DateDebut.ToString("yyyy-MM-dd HH:mm:ss"),
                    end = x.DateFin.ToString("yyyy-MM-dd HH:mm:ss"),
                    content = "<div style=\"color:white;text-align:center;\" class=\"jourferie " + x.StatutString +
                              "\" title=\"" + x.Type.GetDisplayName() + "\">" +
                              x.Type.GetAttribute<CodeAttribute>().Code + "</div>",
                    group = x.UserId
                }).ToList();
            var groups = data
                .Select(x => new
                {
                    id = x.UserId,
                    content = x.Demandeur
                })
                .Distinct();

            var postData = new
            {
                items,
                groups
            };

            return Json(postData);
        }

In my script I replaced

       document.addEventListener("DOMContentLoaded", function () {
            const data = {
                users: []
            };
            fetch("/DemandeConge/GetDataTimeline/",
                {
                    method: "POST",
                    headers: {
                        'Content-Type': "application/json"
                    },
                    body: JSON.stringify(data)
                })
                .then(response => response.json())
                .then(result => {
                    timeline.setData(result); 
                })
                .catch(error => {
                    console.log(error);
                });
        });

by

       document.addEventListener("DOMContentLoaded", function () {
            const data = {
                users: []
            };
            fetch("/DemandeConge/GetDataTimeline/",
                {
                    method: "POST",
                    headers: {
                        'Content-Type': "application/json"
                    },
                    body: JSON.stringify(data)
                })
                .then(response => response.json())
                .then(result => {
                    //timeline.setData(result);
                  
                    var items = [];
                    result.items.forEach(function (item) {
                        const content = document.createElement("div");
                        content.innerHTML = item.content;
                        items.push({
                            start: item.start,
                            end: item.end,
                            content:content,
                            group:item.group
                        });
                    });
                   
                    timeline.setData({
                        groups: result.groups,
                        items:items
                    });
                })
                .catch(error => {
                    console.log(error);
                });
        });

Could be cleaner but it's working for now ;)
Thanks again !
John

[frankcs]

Same situation here. Thanks @jamiegau for the workaround (pinning to 7.4.3). I hope they fix this in a new release. I think it should be developer responsibility to sanitize the content passed to the item. At least some attributes should be allowed. In my case class attributes are being stripped.

[jamiegau]

It's an interesting issue. I can see why they implemented this change. I am waiting for the developers behind this to decide on the solid path forward on this issue. I can go back and, for example, implement a sanitized method and pass that to the item. But I am just going to sit with pinning to the 7.4.3 version until this issue is closed with a documented path with the expected way to implement this capability going forward. Then I will go back and update my code to implement the "documented" way to do this. As so, the issue is unlikely to hit me again in the fuure as they may change their mind etc.

For example, once this item is closed AND they update the demos that implement the custom HTML in an time line item.. (Which are currently broken based on this) I will take that as a sign its time to look at this again and implement whatever is needed to fix functionality.

[yotamberk]

Hi everybody, I took a few days to think about this issue and after a small research in other similar libraries that allow injecting html dangerously, I think the best solution is as follows:

First thig, revert to the original behavior. Introducing the XSS protection is a major breaking change. Larger than I even expected...

After that, we need to reconsider what to do next.
I think we shouldn't allow injecting html just as is. This is an XSS issue and makes this library volnurable.
BUT on the otherhand, we should allow the use to accept dangerously injecting html and take care of this volurability by themselves!

I offer to add an addition option to each of these template options allowing to set dangerously_inject_html: true so we have no doubt the user knows that he chose to deal with this volnurability by himself.

What do you think? @Thomaash @mojoaxel

[mojoaxel]

I'm really sorry that I caused so much problems with my untested quick fix! 😓

I'm not going to co contribute anymore and trust that you guys will find a good solution.

But please be aware that security researchers are monitoring this library closely!

[yotamberk]

It's not your fault. I accepted this fix so the blame is mine just as much. I will revert this change and we'll discuss how to implement this in a more controlled way.

[jamiegau]

@mojoaxel No, it was a great addition. Security is very important. It's just a set of unfortunate events.
My position on this is to allow the old behaviour but fire deprecated warnings, Insecure warnings in the debug pointing to using a more secure NEW method. Old code that may never get fixed will still work, but responsible developers can move forward to the more secure method as needed. (With a lot of javascript debug annoying them until they do...)

[frankcs]

@yotamberk @mojoaxel There is an options parameter in xss that could be exposed somehow to give some flexibility to the user. In that way it can be made secure and flexible.

[YoannChabert]

@mojoaxel Don't be so rude with yourself ! It's a great one ! Better having a design issue than a security one !

[danisss9]

@yotamberk @mojoaxel There is an options parameter in xss that could be exposed somehow to give some flexibility to the user. In that way it can be made secure and flexible.

Yes, this would be very useful to keep security and compatibility with existing code.

[dennissterzenbach]

Hey guys I am happy to read you are discussing the things here. Are there any news on when you will provide a new version (preferably published to npm) which allows to incorporate HTML again?

We're building a fairly complex application and the XSS filters cause some major issues for us, which circumventing does not turn to do a lot for performance. Unfortunately pinning an older version of the library makes our security monitoring go crazy, so that is no option.

So I would be happy to keep in touch here and see more news :-)

[dennissterzenbach]

Hello again, everything starts with transparency, so I just wanted to let you know:
I am now diving into this a bit deeper and try to get a working proposal together to solve (or at least workaround) this issue.
I hope to have a PR ready within the next days, so you can check what I did.

[vis-bot]

🎉 This issue has been resolved in version 7.4.7 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[RonRofe]

#1010 Solution