rstudio / rstudio

RStudio is an integrated development environment (IDE) for R
https://posit.co/products/open-source/rstudio/
Other
4.65k stars 1.09k forks source link

XSS and other weirdness in dataframe viewer with both rownames and list or dataframe entries #10712

Open FallingPineapples opened 2 years ago

FallingPineapples commented 2 years ago

System details

RStudio Edition : Desktop [Open Source]
RStudio Version : 2022.2.0.443
OS Version      : Windows 10 x64 (build 19043)
R Version       : R version 4.0.5 (2021-03-31)

Steps to reproduce the problem

After running the following code, clicking the magnifying glass to open the nested list will cause a Javascript alert:

dt = data.frame(a = 0, row.names = 'alert(1)')
dt$a = list(list())
View(dt)

Running this code will cause two Javascript alerts:

dt = data.frame(a = 0, row.names = '"><img src onerror="alert(\'Hello!\')">')
dt$a = list(list())
View(dt)

Describe the problem in detail

The problematic line is here: https://github.com/rstudio/rstudio/blob/74512a0ea3c92a8f10f1614dbc83d40bf5ad8e24/src/cpp/session/resources/grid/dtviewer.js#L332 The row name here is put directly into the HTML.

Describe the behavior you expected

I would expect this to use the row number instead of the row name. I don't know if this is available as commit e69526bf2dc96ea28c3151a3a99cb164c3969776 (part of the pull request that introduced the feature) intentionally changed this, possibly having some issues with the virtualized rows.

ronblum commented 2 years ago

@FallingPineapples Thank you for raising the issue, and I especially appreciate your thorough investigation!

I'm able to consistently reproduce this going back to RStudio Desktop 1.2.5042 on MacOS 12.1. We'll review this as we continue development of RStudio.

Triage note: This isn't a regression, so I'm marking this as a candidate for the Data Viewer redesign.