Modifying POST request payload

[ABM893]

Summary

This line of code will display an error in a user's browser if I modify the request body payload by altering the names used in the POST request.

    res = tryCatch(filter(data, params), error = function(e) {
      list(error = as.character(e))
    })

For context we had this line cause a failure in - I believe - pen testing software on the grounds it's leaking the application code. I would argue that but won't here. It feels like the above could be passed back to my R session in the form of an error that I can sanitize when in a Shiny app. Instead of as is, where it's passed to toJSON and displayed as a JSON error.

Steps to reproduce:

• Using this example app
• Modify the request body - in Firefox, right click and 'Inspect Element '
• Select the POST request for the table, begins 'x3?w='
• Click on the 'Edit and Resend' button
• Replace 'searchable' with 'isnotsearchable' or any string other than 'searchable'
• Click on 'Send' and go to the Response tab
• The response will be a JSON error "Error in if (col[["searchable"]] != "true") next: argument is of length zero"
• Note a similar error will result for other named elements used in logical operations in dataTablesFilter, for example: if ((k <- col[['search']][['value']]) == '') next

---

By filing an issue to this repo, I promise that

• I have fully read the issue guide at https://yihui.name/issue/.
• I have provided the necessary information about my issue.
  • If I'm asking a question, I have already asked it on Stack Overflow or RStudio Community, waited for at least 24 hours, and included a link to my question there.
  • If I'm filing a bug report, I have included a minimal, self-contained, and reproducible example, and have also included xfun::session_info('DT'). I have upgraded all my packages to their latest versions (e.g., R, RStudio, and R packages), and also tried the development version: remotes::install_github('rstudio/DT').
  • If I have posted the same issue elsewhere, I have also mentioned it in this issue.
• I have learned the Github Markdown syntax, and formatted my issue correctly.

I understand that my issue may be closed if I don't fulfill my promises.

[akgold]

Hi,

@ABM893 to be clear of the vulnerability here, the lines that result in the issue are https://github.com/rstudio/DT/blob/master/R/shiny.R#L514-L515, and the lines being leaked are https://github.com/rstudio/DT/blob/master/R/shiny.R#L569-L570.

Is that correct?

[ABM893]

Hi,

@akgold that's correct.

[jcheng5]

It's not the same as full error sanitizing, but replacing:

list(error = as.character(e))

with

list(error = conditionMessage(e))

would at least get rid of the "Error in xxx:" part of the message, which seems to be the cause of most of the concern?

A more comprehensive fix would also need:

1. if getOption("shiny.sanitize.errors", FALSE) && !inherits(e, "shiny.custom.error") is TRUE, return a hardcoded generic error message (this should really be made available as an exported function from Shiny).
2. Log the full error to console--or better yet, shiny::withLogErrors().

[ABM893]

would at least get rid of the "Error in xxx:" part of the message, which seems to be the cause of most of the concern?

@jcheng5 It would be better, the hard-coded message you propose being better still. I've always liked the behaviour of shiny.sanitize.errors. In that it's a helpful safety net with the ability to add a finer level of control on top of it if needed.

Thank you both for your quick response