qs icon indicating copy to clipboard operation
qs copied to clipboard

Deserialization arbitrary code execution attack

Open thomascwells opened this issue 1 year ago • 12 comments

Is the qs format susceptible to the same type of deserialization attack underlying the recent RDS CVE?

References:

  • https://support.posit.co/hc/en-us/articles/23170092899607-CVE-2024-27322-R-bitrary-Code-Execution
  • https://www.kb.cert.org/vuls/id/238194
  • https://hiddenlayer.com/research/r-bitrary-code-execution/

thomascwells avatar May 01 '24 15:05 thomascwells

Thank you, I will look into it.

traversc avatar May 01 '24 16:05 traversc

FWIW - AFAICT yes. You can use qs to replicate the following example given by George Stagg in https://mstdn.social/@gws/112359739655466497

TimTaylor avatar May 01 '24 21:05 TimTaylor

Updated on github. R 4.4 throws an error if you try to unserialize a promise. I actually think that's the wrong approach, instead I replace it with NULL and issue a warning.

e <- new.env()
delayedAssign("x", system("uname -msrn"), assign.env = e)
qsave(e, "/mnt/n/temp.qs")
qread("/mnt/n/temp.qs")$x
# Warning message:
#   In qread("/mnt/n/temp.qs") :
#   PROMSXP detected, replacing with NULL (see https://github.com/traversc/qs/issues/93)

Editorial: I agree with gws on mastadon, I don't think this is a big deal, the issue is equivalent to if you ran source on an unknown file. Make sure you know where your files come from!

traversc avatar May 02 '24 06:05 traversc

Editorial: I agree with gws on mastadon, I don't think this is a big deal, the issue is equivalent to if you ran source on an unknown file. Make sure you know where your files come from!

Agree completely. Mainly came to flag for awareness and saw the issue already.

TimTaylor avatar May 02 '24 06:05 TimTaylor

On CRAN now

traversc avatar May 16 '24 15:05 traversc

Hm, I understand the security concern, but the patch is breaking a few hundred ggplot2/ggiraph objects stored to disk being used in our reports (longer story why we pre-store these to disk). Except for locally rolling back to qs 0.26.1 (0.26.2 is not in the CRAN archive?), is there a way to avoid replacing the PROMSXP with NULL if I trust these files? An override_safety argument perhaps? The gg objects would also be somewhat cumbersome to reproduce and store as new file (formats) now in our production process.

sda030 avatar May 29 '24 13:05 sda030

An override argument sounds reasonable.

traversc avatar May 30 '24 07:05 traversc

@sda030 Try the latest commit with your previous ggplots and let me know if it works.

New function set_trust_promises

Standard behavior: evaluate promises during save, don't allow promises during read.

e <- new.env()
delayedAssign("x", system("uname -msrn"), assign.env = e)

qsave(e, "/mnt/n/temp.qs")
# Linux Travers-PC 5.15.146.1-microsoft-standard-WSL2 x86_64

qread("/mnt/n/temp.qs")$x
[1] 0

With set_trust_promises

e <- new.env()
delayedAssign("x", system("uname -msrn"), assign.env = e)

set_trust_promises(TRUE)
qsave(e, "/mnt/n/temp.qs")

qread("/mnt/n/temp.qs")$x
# Linux Travers-PC 5.15.146.1-microsoft-standard-WSL2 x86_64
# [1] 0

traversc avatar Jun 05 '24 05:06 traversc

Thanks for quick response @traversc! And sorry for my late response. It works for your example. Tough a bit weird that when setting set_trust_promises(TRUE), I get a FALSE in return? Should not the returned value be the value being changed to, not the value it was? (Alternatively consider a more informative "Previous value: FALSE") Very much like the idea of a global option. However, this setting only applies before saving files, which is a bit late for my already saved plots..?

P.S. Will completely change my setup this summer to avoid pre-storing the plots for the future.

sda030 avatar Jun 09 '24 21:06 sda030

The global option changes both saving and reading files, so you should be able to read previously saved promises from earlier versions of R / qs.

traversc avatar Jun 10 '24 01:06 traversc

I swear it did not work the first time, but maybe I missed a step. Now it works consistently, also with reading in a ggobj. I also tested turning it off and on again. Nice! Thank you very much for swift action. :)

sda030 avatar Jun 10 '24 07:06 sda030

@traversc, may I politely request a patch release on CRAN before August? (I try to protect my less-technically oriented colleagues from R's many ideosyncracies such as Rtools, devtools::install_github(), etc)

sda030 avatar Jun 12 '24 08:06 sda030