Deserialization arbitrary code execution attack

[thomascwells]

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/

[traversc]

Thank you, I will look into it.

[TimTaylor]

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

[traversc]

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!

[TimTaylor]

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.

[traversc]

On CRAN now

[sda030]

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.

[traversc]

An override argument sounds reasonable.

[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

[sda030]

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.

[traversc]

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.

[sda030]

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]

@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)

[traversc]

Fixed with latest update on CRAN