Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[CVE-2020-24164] Remote Code Execution (RCE) vulnerability via Java's Serializable interface #130

Closed
ptaoussanis opened this issue Jul 24, 2020 · 5 comments
Assignees
Labels

Comments

@ptaoussanis
Copy link
Member

ptaoussanis commented Jul 24, 2020

SECURITY ADVISORY [CVE-2020-24164]

This vulnerability kindly pointed out in an excellent report by @solita-timo-mihaljov. Huge thanks to Timo!

Quick summary

  • Nippy versions from v2.5.0-beta1 (24 Oct 2013) and before v2.15.0 final (24 Jul 2020) contain an RCE (Remote Code Execution) vulnerability that may allow an attacker to execute arbitrary code when thawing a malicious payload controlled by the attacker.

  • Similar vulnerabilities in the Jackson library have been given "High" CVE scores of 7.5 out of 10.

Who is affected?

You are vulnerable if the following conditions all hold:

  • You are thawing (deserializing) data from an untrusted source (usually not a great idea in general), or an attacker is somehow able to inject data into a trusted source.
  • You are using a Nippy version from v2.5.0-beta1 (Oct 2013) through v2.15.0-RC1.
  • You have vulnerable "gadget" classes on your classpath that can be combined into a "gadget chain".
    • Important: Clojure <= v1.8 itself unfortunately includes such a chain.
    • Many other libraries do too, some examples here.

Attack description

  1. Attacker creates a malicious payload: an object of specific vulnerable class/es, which include malicious code.
  2. Attacker manages to get the system to freeze and then thaw this malicious payload.
  3. On thawing, the malicious arbitrary code is run within the same JVM process.

How was the vulnerability introduced?

With commit 9448d2b (24 Oct 2013), Nippy introduced a feature to allow the automatic use of Java's Serializable interface as a fallback for types that Nippy didn't support via its own Freezable protocol.

Unfortunately I wasn't aware of the concept of a gadget-chain attack until this report.

Mitigation

Upgrade to Nippy >= v2.15.0 final. Always prefer the latest stable version when possible.


Upgrade instructions

Upgrading to v3 from earlier versions

v3 is safe by default, and often relatively painless to upgrade to.

Default v3 behaviour:

  • freeze will continue to allow any Serializable class to be frozen, as in older vulnerable versions of Nippy.
  • When thaw encounters a disallowed Serialized class, it will:
    • Throw if the object was frozen with Nippy < v2.15.0 final.
    • Otherwise it will return a safely quarantined object of form {:nippy/unthawable {:class-name <> :content <quarantined-ba>}.
    • The default thaw allowlist contains a number of common safe class names. See its value for details.

Quarantined objects may:

  • Be manually unquarantined with read-quarantined-serializable-object-unsafe!.
  • Themselves be safely frozen, thawed, transported, etc.

Customising v3 behaviour:

There are two relevant allowlists for you to configure:

See their docstrings for detailed configuration info and example values.

In most cases, you'll want to leave *freeze-serializable-allowlist* to its default (permissive) value, then customize *thaw-serializable-allowlist* based on your risk profile and needs:

  • If you never thaw untrusted payloads (ideal goal whenever possible): a permissive *thaw-serializable-allowlist* like {"*"} would be fine.
  • If you may thaw untrusted payloads: consider expanding the default set of allowed classes as necessary.

Upgrading to v2.5 from earlier versions

v2.5 is safe by default, but can be somewhat painful to upgrade to due to the risk of freeze throwing in cases where it previously didn't throw. Prefer upgrading to v3 if possible.

Default v2.5 behaviour:

  • When freeze encounters a disallowed Serialized class, it will throw.
  • When thaw encounters a disallowed Serialized class, it will:
    • Throw if the object was frozen with Nippy < v2.15.0 final.
    • Otherwise it will return a safely quarantined object of form {:nippy/unthawable {:class-name <> :content <quarantined-ba>}.
    • The default allowlist may contain a number of common safe class names. See its value for details.

Customising v2.5 behaviour:

See *serializable-whitelist* docstring for detailed configuration info and example values.

Questions? Suggestions?

Please feel free to comment on this thread. Will make a best effort to prioritise responses on this topic.

@ptaoussanis ptaoussanis self-assigned this Jul 24, 2020
@ptaoussanis ptaoussanis pinned this issue Jul 24, 2020
@glenjamin
Copy link

Is there a good way to get 2.14.2 to spit out a list of types it's seen over time?

@ptaoussanis
Copy link
Member Author

ptaoussanis commented Jul 25, 2020

Hi @glenjamin, very nice idea!

The code snippets below will allow any class to use Nippy's Serializable support, and record its class name. You can use these snippets while transitioning from a vulnerable to safe configuration. They'll let you see which classes Nippy has been using Serializable for under-the-covers.

If you're satisfied that all the recorded classes are safe, you can then add them to Nippy's default allowlist.

Example for v3:

See also allow-and-record-any-serializable-class-unsafe docstring for more info.

(alter-var-root #'*freeze-serializable-allowlist* (fn [_] "allow-and-record"))
(alter-var-root   #'*thaw-serializable-allowlist* (fn [_] "allow-and-record"))

(comment (get-recorded-serializable-classes)) ; Call/log after some time

(comment
  ;; If you're satisfied that the recorded classes are safe, you can merge them
  ;; into Nippy's default allowlist:
  (alter-var-root #'thaw-serializable-allowlist*
    (fn [_] (into default-thaw-serializable-allowlist
              (keys (get-recorded-serializable-classes)))))

Example for v2.15 or v2.14.2:

;; Deref for set of all class names that made use of Nippy's Serializable support:
(defonce observed-serializables_ (atom #{}))

(swap-serializable-whitelist!
  (fn [_]
    (fn allow-classname? [class-name]
      (swap! observed-serializables_ conj class-name) ; Record class
      true ; Allow any class
      )))

(comment @observed-serializables_) ; Call/log after some time

(comment
  ;; If you're satisfied that the recorded classes are safe, you can merge them
  ;; into Nippy's default allowlist:
  (swap-serializable-whitelist!
    (fn [_] (into default-serializable-whitelist observed-serializables_))))

Cheers!

@ptaoussanis
Copy link
Member Author

Quick update: keeping this open since I'm still waiting for a CVE ID to be issued.
Just pinged the folks at cve.mitre.org to see if there's been any follow-up on the request.

ptaoussanis added a commit that referenced this issue Sep 10, 2020
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.
ptaoussanis added a commit that referenced this issue Sep 10, 2020
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.
ptaoussanis added a commit that referenced this issue Sep 10, 2020
…parate freeze/thaw lists

Removed 2x vars:
  -     *serializable-whitelist*
  - swap-serializable-whitelist!

Added 4x vars:
  -     *freeze-serializable-allowlist*
  -       *thaw-serializable-allowlist*
  - swap-freeze-serializable-allowlist!
  -   swap-thaw-serializable-allowlist!

Deprecated 2x JVM properties:
  - taoensso.nippy.serializable-whitelist-base
  - taoensso.nippy.serializable-whitelist-add

Deprecated 2x ENV vars:
  - TAOENSSO_NIPPY_SERIALIZABLE_WHITELIST_BASE
  - TAOENSSO_NIPPY_SERIALIZABLE_WHITELIST_ADD

API is otherwise identical.

MOTIVATION

  An API break is unfortunate- but the break here is small, and the
  benefit significant.

  By separating the freeze/thaw lists, it becomes possible to safely
  allow *any* classes to be frozen - and so effectively make the
  allowlist a purely thaw-time concern in the common case.

  This has several advantages including:

    - No risk of Nippy calls unexpectedly throwing where they didn't
      before.

    - The ability to adjust or bypass the thaw allowlist *after*
      seeing which class objects have been quarantined.

  In general: this change eases migration to RCE-safe Nippy from
  RCE-vulnerable versions. This is especially useful in cases where
  Nippy is being used as an ~implementation detail for another
  library/application/service.
@ptaoussanis ptaoussanis changed the title Remote Code Execution vulnerability via Java's Serializable interface [CVE-2020-24164] Remote Code Execution (RCE) vulnerability via Java's Serializable interface Sep 11, 2020
ptaoussanis added a commit that referenced this issue Sep 11, 2020
…parate freeze/thaw lists

Removed 2x vars:
  -     *serializable-whitelist*
  - swap-serializable-whitelist!

Added 4x vars:
  -     *freeze-serializable-allowlist*
  -       *thaw-serializable-allowlist*
  - swap-freeze-serializable-allowlist!
  -   swap-thaw-serializable-allowlist!

Deprecated 2x JVM properties:
  - taoensso.nippy.serializable-whitelist-base
  - taoensso.nippy.serializable-whitelist-add

Deprecated 2x ENV vars:
  - TAOENSSO_NIPPY_SERIALIZABLE_WHITELIST_BASE
  - TAOENSSO_NIPPY_SERIALIZABLE_WHITELIST_ADD

API is otherwise identical.

MOTIVATION

  An API break is unfortunate- but the break here is small, and the
  benefit significant.

  By separating the freeze/thaw lists, it becomes possible to safely
  allow *any* classes to be frozen - and so effectively make the
  allowlist a purely thaw-time concern in the common case.

  This has several advantages including:

    - No risk of Nippy calls unexpectedly throwing where they didn't
      before.

    - The ability to adjust or bypass the thaw allowlist *after*
      seeing which class objects have been quarantined.

  In general: this change eases migration to RCE-safe Nippy from
  RCE-vulnerable versions. This is especially useful in cases where
  Nippy is being used as an ~implementation detail for another
  library/application/service.
@ptaoussanis
Copy link
Member Author

CVE-2020-24164 was issued yesterday, closing.

ptaoussanis added a commit that referenced this issue Sep 12, 2020
A convenience for folks upgrading from older versions of Nippy
still vulnerable to #130.
@mpenet mpenet unpinned this issue Sep 12, 2020
@mpenet mpenet pinned this issue Sep 12, 2020
@mpenet
Copy link
Collaborator

mpenet commented Sep 12, 2020

Sorry about the pinning toggle! I thought it was just local to my session, that also reminded me I have contrib access here I think.

edporras added a commit to ngrunwald/datasplash that referenced this issue Mar 23, 2021
Versions after 2.14.0 required a breaking change due to security
advisories (see taoensso/nippy#130). The
fix requires explicit declaration of the types that are safe to
thaw.
edporras added a commit to edporras/datasplash that referenced this issue Aug 13, 2021
Versions after 2.14.0 required a breaking change due to security
advisories (see taoensso/nippy#130). The
fix requires explicit declaration of the types that are safe to
thaw.
edporras added a commit to edporras/datasplash that referenced this issue Aug 13, 2021
Versions after 2.14.0 required a breaking change due to security
advisories (see taoensso/nippy#130). The
fix requires explicit declaration of the types that are safe to
thaw.
ngrunwald pushed a commit to ngrunwald/datasplash that referenced this issue Aug 14, 2021
Versions after 2.14.0 required a breaking change due to security
advisories (see taoensso/nippy#130). The
fix requires explicit declaration of the types that are safe to
thaw.
@ptaoussanis ptaoussanis unpinned this issue Jul 27, 2023
Frozenlock added a commit to Frozenlock/codax that referenced this issue Aug 12, 2023
"Nippy versions from v2.5.0-beta1 (24 Oct 2013) and before v2.15.0 final (24 Jul 2020) contain an RCE (Remote Code Execution) vulnerability that may allow an attacker to execute arbitrary code when thawing a malicious payload controlled by the attacker."

taoensso/nippy#130
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants