Serializable: NB freeze default is now always to ALLOW ALL

We have 2 options:

  A: Default to Serializable whitelist checks on both freeze and thaw
  B: Default to Serializable whitelist checks only on thaw

Before this commit, Nippy was taking option A.
As of  this commit, Nippy is  taking option B.

Both are equally safe re: the risk of Remote Code Execution in #130:

  - Freezing a        malicious payload  is  *not* a security risk
  - Thawing  a frozen malicious payload *is*       a security risk.

But option B has the benefit of not throwing exceptions by default
against a whitelist that has not [yet] been properly configured.

This is especially helpful for other libraries or applications that
may be using Nippy as an underlying dependency.

Behaviour under our two options against a whitelist that has not
[yet] been properly configured:

  A: Throw exception on freeze
  B: Freeze successfully, and thaw successully as
     {:nippy/unthawable {:class-name <> :content <quarantined-ba> :cause :quarantined}}

I think this is probably less of a nuissance, and so a better default.
This commit is contained in:
Peter Taoussanis 2020-09-10 13:35:55 +02:00
parent db71943a5b
commit db2c22eed8
2 changed files with 28 additions and 18 deletions

View file

@ -1196,21 +1196,28 @@
- :encryptor nil - :encryptor nil
- :no-header? true" - :no-header? true"
[x] [x]
(let [baos (ByteArrayOutputStream. 64) (binding [*serializable-whitelist* "*"]
dos (DataOutputStream. baos)] (let [baos (ByteArrayOutputStream. 64)
(with-cache (-freeze-with-meta! x dos)) dos (DataOutputStream. baos)]
(.toByteArray baos))) (with-cache (-freeze-with-meta! x dos))
(.toByteArray baos))))
(defn freeze (defn freeze
"Serializes arg (any Clojure data type) to a byte array. To freeze custom "Serializes arg (any Clojure data type) to a byte array. To freeze custom
types, extend the Clojure reader or see `extend-freeze`." types, extend the Clojure reader or see `extend-freeze`.
Note: `serializable-whitelist` is \"*\" by default for freezing (not thawing!).
If you want to use the value of `*serializable-whitelist*` instead, include
`:serializable-whitelist :default` in opts."
([x] (freeze x nil)) ([x] (freeze x nil))
([x {:as opts ([x {:as opts
:keys [compressor encryptor password serializable-whitelist incl-metadata?] :keys [compressor encryptor password serializable-whitelist incl-metadata?]
:or {compressor :auto :or {compressor :auto
encryptor aes128-gcm-encryptor}}] encryptor aes128-gcm-encryptor
serializable-whitelist "*"}}]
(call-with-bindings opts (call-with-bindings (assoc opts :serializable-whitelist serializable-whitelist)
(fn [] (fn []
(let [;; Intentionally undocumented: (let [;; Intentionally undocumented:

View file

@ -252,26 +252,27 @@
(is (is
(thrown? Exception (thrown? Exception
(binding [nippy/*serializable-whitelist* #{}] (nippy/freeze (java.util.concurrent.Semaphore. 1)
(nippy/freeze (java.util.concurrent.Semaphore. 1)))) {:serializable-whitelist #{}}))
"Can't freeze Serializable object unless approved by whitelist") "Can't freeze Serializable object unless approved by whitelist")
(is (is
(:nippy/unthawable (:nippy/unthawable
(let [ba (binding [nippy/*serializable-whitelist* #{"java.util.concurrent.Semaphore"}] (let [ba (nippy/freeze (java.util.concurrent.Semaphore. 1)
(nippy/freeze (java.util.concurrent.Semaphore. 1)))] {:serializable-whitelist #{"java.util.concurrent.Semaphore"}})]
(binding [nippy/*serializable-whitelist* #{}] (nippy/thaw ba {:serializable-whitelist #{}})))
(nippy/thaw ba))))
"Can't thaw Serializable object unless approved by whitelist") "Can't thaw Serializable object unless approved by whitelist")
(is (is
(instance? java.util.concurrent.Semaphore (instance? java.util.concurrent.Semaphore
(binding [nippy/*serializable-whitelist* #{"java.util.concurrent.Semaphore"}] (nippy/thaw
(nippy/thaw (nippy/freeze (java.util.concurrent.Semaphore. 1))))) (nippy/freeze (java.util.concurrent.Semaphore. 1)
{:serializable-whitelist #{"java.util.concurrent.Semaphore"}})
{:serializable-whitelist #{"java.util.concurrent.Semaphore"}}))
"Can freeze and thaw Serializable object if approved by whitelist") "Can freeze and thaw Serializable object if approved by whitelist")
@ -280,7 +281,7 @@
(get-in (get-in
(nippy/thaw (nippy/thaw
(nippy/freeze (java.util.concurrent.Semaphore. 1) (nippy/freeze (java.util.concurrent.Semaphore. 1)
{:serializable-whitelist "*"}) #_{:serializable-whitelist "*"})
{:serializable-whitelist #{}}) {:serializable-whitelist #{}})
[:nippy/unthawable :cause])) [:nippy/unthawable :cause]))
@ -288,8 +289,10 @@
(is (is
(instance? java.util.concurrent.Semaphore (instance? java.util.concurrent.Semaphore
(binding [nippy/*serializable-whitelist* #{"java.util.concurrent.*"}] (nippy/thaw
(nippy/thaw (nippy/freeze (java.util.concurrent.Semaphore. 1))))) (nippy/freeze (java.util.concurrent.Semaphore. 1)
{:serializable-whitelist #{"java.util.concurrent.*"}})
{:serializable-whitelist #{"java.util.concurrent.*"}}))
"Strings in whitelist sets may contain \"*\" wildcards") "Strings in whitelist sets may contain \"*\" wildcards")