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

Protect against RCE vulnerability in Nippy #30

Merged
merged 1 commit into from
Sep 7, 2023

Conversation

Frozenlock
Copy link
Contributor

"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

"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
@dscarpetti
Copy link
Owner

Thanks for the PR. This definitely needs to be addressed. My concern is that it may (partially) break backwards compatibility with previously made codax databases. Data would still be safely stored, but unexpected quarantine values might result when decoded. If I am understanding the changes in v3 of nippy correctly the potential issue is with user defined types which may need to be included in nippy's *thaw-serializable-allowlist*? I'm not totally clear on all the implications in practice. The only non-standard type supported by codax is org.joda.time.DateTime which seems to freeze/thaw without issue though org.joda.time.Instants do not. (of course, joda-time is pretty old and a future major version of codax could probably drop support.)

It may be as simple as including a helper function:

(defn whitelist-value-types!
  "specify additional types for paths and values that are safe to decode" 
  [& ts]
  (alter-var-root #'taoensso.nippy/*thaw-serializable-allowlist*
    (fn [allowlist] (into allowlist ts)))

@Frozenlock
Copy link
Contributor Author

Should nippy abstracted away? The readme gives the impression that codax relies on nippy and that the user should be aware of that. In that case, I would expect users to use *thaw-serializable-allowlist* directly. If it is abstracted, then perhaps the types could be added automatically when adding custom types? It could also take care of org.joda.time.Instant at the same time, when it's defined with defpathtype (assuming it survives the next version bump).

@dscarpetti dscarpetti merged commit 92f70d7 into dscarpetti:master Sep 7, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants