Security: DataTables.net prototype pollution

[mukesh08592]

System details

Browser Version:

Output of sessionInfo():

# sessionInfo() output goes here

Example application or steps to reproduce the problem

# Minimal, self-contained example app code goes here

Describe the problem in detail

EXPLANATION The datatables.net package is vulnerable to Prototype Pollution. The setData function in jquery.dataTables.js fails to protect prototype attributes when objects are created during the application's execution. A remote attacker can exploit this to modify the behavior of object prototypes which, depending on their use in the application, may result in a Denial of Service (DoS), Remote Code Execution (RCE), or other unexpected execution flow.

DETECTION The application is vulnerable by using this component.

ROOT CAUSE shiny_1.7.1.tar.gzshiny/inst/www/shared/datatables/js/jquery.dataTables.min.js( ,1.10.13) shiny_1.7.1.tar.gzshiny/inst/www/shared/datatables/js/jquery.dataTables.min.js(,)

[tyner]

Yes, it would be good to get some clarity on this. In the meantime our only option is to downgrade back to shiny version 1.5.0 (the most recent version not flagged for the vulnerability).

[wch]

Here's what I can find about this issue: https://security.snyk.io/vuln/SNYK-JS-DATATABLESNET-598806

I believe that it's only a problem if it's running DataTables in a NodeJS server environment (which we are not doing). From that page:

The following environments are susceptible to a Prototype Pollution attack:

• Application server
• Web server

That said, it's a good idea to update it anyway.

[hedsnz]

@wch Would you accept a PR for this update? Looks like it was fixed in DatTables 1.10.22: https://github.com/DataTables/Dist-DataTables/commit/e2e19eac7e5a6f140d7eefca5c7deba165b357eb

[wch]

Just a heads up: The DT package already has a newer version of the DataTables JavaScript file, so if you use that instead of the (semi-deprecated) version built into shiny, this shouldn't be an issue.

To use the version from DT, call DT::renderDataTable, instead of shiny::renderDataTable.

@hedsnz A PR would be helpful, thanks!

[jcheng5]

@wch It's been semi-deprecated since 2015... at what point do we just remove those functions (or have them simply error with a pointer to DT)?

[jcheng5]

BTW, I recommend you use DT::renderDT and DT::DTOutput; they do the same as DT::renderDataTable and DT::datatTableOutput, but less likely to lead to confusion due to being named differently than the shiny versions.

[wch]

@jcheng5 Unfortunately, the deprecation message currently only prints when in dev mode. So it would be a good idea to first make it print the deprecation message without dev mode, and then at some point in the future remove the function entirely.

[wch]

There's a fair amount of code out there that explicitly calls shiny::renderDataTable, and probably even more that doesn't use shiny::.

I just had another idea, though. Maybe we could have shiny::renderDataTable simply call DT::renderDataTable, and print a deprecation message.

[tyner]

Absolutely in favor of deprecating and removing obsolete code. Especially if it gets shiny out of the doghouse.

[jcheng5]

@wch Incredibly, shiny::renderDataTable and DT::renderDataTable are not compatible. Although I think the vast majority of cases can be automatically converted from the former to the latter without much trouble.

[foobar0000]

Hi all. I am also having this issue with this prototype pollution that is affecting datatables.net. Currently, I cannot use any shiny version because of this issue.

Just a heads up: The DT package already has a newer version of the DataTables JavaScript file, so if you use that instead of the (semi-deprecated) version built into shiny, this shouldn't be an issue.

If so, would it be possible to remove the package definition from package.json#L24?

Otherwise, if we need to keep it in package.json, would it be possible to upgrade the version there and use another one that does not contain this bug?

[hedsnz]

@foobar0000 If you need immediate mitigation to satisfy security concerns, using DT::renderDT instead of shiny::renderDataTable will use the patched version of DataTables without the vulnerability.

I can't speak for the Shiny devs, but my reading of the above conversation is that ultimately, the DataTables dependency should be removed from Shiny, given that DT already packages a more up-to-date version. However, more time is needed to warn users to replace shiny::renderDataTable before that happens.

The first step is therefore to print the deprecation message by default, which I've made a PR for (#3718). Then, in a subsequent release (ideally only a short time later), the DataTables dependency (and hence the shiny::renderDataTable function) can be removed entirely.

[jonthegeek]

The description of the Superseded label might make things a bit tricky/confusing:

Superseded functions will not receive new features, but will receive any critical bug fixes needed to keep it working. In some ways a superseded function is actually safer than a stable function because it’s guaranteed never to change (for better or for worse).

Making shiny::renderDataTable call DT::renderDataTable ~silently when possibly might be the best option that won't make anybody mad.

[hedsnz]

So there are two alternatives, each with problems:

1. Have shiny::renderDataTable call DT::renderDT. Con: The two functions are not always compatible.
2. Add the deprecated label to shiny::renderDataTable for a period of time (as per https://github.com/rstudio/shiny/pull/3718), then eventually remove it entirely. Con: shiny::renderDataTable has already been labelled as superseded, implying that it will always be available.

I don't know the details of the potential incompatibility between DT::renderDT and shiny::renderDataTable, but unless there is a simple way of determining whether that wrapper will work, my preference is for 2. Are we able to get a decision from the Shiny devs @jcheng5 ? Thanks.

[tyner]

Any update on this? If not, when might we reasonably expect resolution?