Skip to content

Commit

Permalink
Upgrade to SnakeYAML 2.1 (#87)
Browse files Browse the repository at this point in the history
* bump snakeyaml to 2.0

* tests: adapt to changes in exception messages

Adapt to changes to exception messages in SnakeYAML 2.0

* unsafe is still globally unsafe

SnakeYAML 2.0 responded to CVE-2022-1471 by disallowing global tags by
default for any usage.

Global tags were never allowed when using the SnakeYAML SafeConstructor
which clj-yaml uses by default. This is why we felt we were not affected
by CVE-2022-1471.

When the user requests `:unsafe` YAML, clj-yaml use the SnakeYAML
Constructor. We adapt to 2.0 changes by continuing to allow globally
unsafe construction under this usage via the TrustedTagInspector, which
allows any and all tags to be created.

For fine grained tag creation permissions, users can opt to use clj-yaml's
`:unknown-tag-fn`.

* configure nvd scan to ignore false positives

I previously determined that 2 false positives did not apply to
SnakeYAML but I configured these false positives to be ignored for v1.33
of SnakeYAML only. Globally ignore them instead to that they apply to
all versions of SnakeYAML.

* Adapt to removal of TrustedTagInspector

SnakeYAML 2.1 will remove TrustedTagInspector.

Do effectively the same work via a reify-ed TagInspector.

* turf accidentally added space

* deps: bump snakeyaml to v2.1

* docs: update changelog
  • Loading branch information
lread authored Aug 10, 2023
1 parent ab98016 commit 2799a9b
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 12 deletions.
12 changes: 7 additions & 5 deletions CHANGELOG.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,17 @@ Clj-yaml makes use of SnakeYAML, please also refer to the https://bitbucket.org/
** Added `:code-point-limit` option to accept bigger documents
(https://github.com/clj-commons/clj-yaml/issues/94[#94])
(https://github.com/pitalig[@pitalig])
* Dependencies
** Bump `org.flatland/ordered` to `1.15.11`
(https://github.com/clj-commons/clj-yaml/issues/98[#98])
(https://github.com/borkdude[@borkdude])
** Bump `org.yaml/snakeyaml` to `2.1`
(https://github.com/clj-commons/clj-yaml/issues/86[#86])
(https://github.com/lead[@lread])
* Quality
** Stop using deprecated SnakeYAML Representer constructor
(https://github.com/clj-commons/clj-yaml/issues/76[#76])
(https://github.com/lead[@lread])

** Bump `org.flatland/ordered` to `1.15.11`
(https://github.com/clj-commons/clj-yaml/issues/98[#98])
(https://github.com/borkdude[@borkdude])

** Add Eastwood linting as lint-eastwood bb task and to CI
(https://github.com/clj-commons/clj-yaml/issues/77[#77])
(https://github.com/lead[@lread])
Expand Down
2 changes: 1 addition & 1 deletion deps.edn
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{:paths ["src/clojure" "target/classes"]
:deps {org.yaml/snakeyaml {:mvn/version "1.33"}
:deps {org.yaml/snakeyaml {:mvn/version "2.1"}
org.flatland/ordered {:mvn/version "1.15.11"}}
:deps/prep-lib {:alias :build
:fn compile-java
Expand Down
2 changes: 0 additions & 2 deletions nvd_check_helper_project/suppressions.xml
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,12 @@
<notes><![CDATA[
False positive, does not apply to snakeyaml at all.
]]> </notes>
<filePath regex="true">.*\bsnakeyaml-1.33.jar</filePath>
<cve>CVE-2022-3064</cve>
</suppress>
<suppress>
<notes><![CDATA[
False positive, does not apply to snakeyaml at all.
]]> </notes>
<filePath regex="true">.*\bsnakeyaml-1.33.jar</filePath>
<cve>CVE-2021-4235</cve>
</suppress>
</suppressions>
10 changes: 8 additions & 2 deletions src/clojure/clj_yaml/core.clj
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
[flatland.ordered.set :refer (ordered-set)])
(:import (org.yaml.snakeyaml Yaml DumperOptions DumperOptions$FlowStyle LoaderOptions)
(org.yaml.snakeyaml.constructor Constructor SafeConstructor BaseConstructor)
(org.yaml.snakeyaml.inspector TagInspector)
(org.yaml.snakeyaml.representer Representer)
(org.yaml.snakeyaml.error Mark)
(clj_yaml MarkedConstructor UnknownTagsConstructor)
Expand Down Expand Up @@ -89,7 +90,7 @@
Returns internal SnakeYAML loader options.
See [[parse-string]] for description of options."
^LoaderOptions [& {:keys [max-aliases-for-collections allow-recursive-keys allow-duplicate-keys nesting-depth-limit code-point-limit]}]
^LoaderOptions [& {:keys [max-aliases-for-collections allow-recursive-keys allow-duplicate-keys nesting-depth-limit code-point-limit unsafe]}]
(let [loader (default-loader-options)]
(when nesting-depth-limit
(.setNestingDepthLimit loader nesting-depth-limit))
Expand All @@ -101,6 +102,10 @@
(.setAllowDuplicateKeys loader allow-duplicate-keys))
(when code-point-limit
(.setCodePointLimit loader code-point-limit))
(when unsafe
(.setTagInspector loader (reify TagInspector
(isGlobalTagAllowed [_this _tag]
true))))
loader))

(defn make-yaml
Expand All @@ -114,7 +119,8 @@
:allow-recursive-keys allow-recursive-keys
:allow-duplicate-keys allow-duplicate-keys
:nesting-depth-limit nesting-depth-limit
:code-point-limit code-point-limit)
:code-point-limit code-point-limit
:unsafe unsafe)
^BaseConstructor constructor
(cond
unsafe (Constructor. loader)
Expand Down
4 changes: 2 additions & 2 deletions test/clj_yaml/core_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -428,10 +428,10 @@ sequence: !CustomSequence
(def dangerous-yaml "!!javax.script.ScriptEngineManager [!!java.net.URLClassLoader [[!!java.net.URL [\"very-bad-badness-here\"]]]]")

(deftest unsafe-deny-test
(is (thrown-with-msg? YAMLException #"(?m).*could not.*constructor.*ScriptEngineManager"
(is (thrown-with-msg? YAMLException #"(?m).*Global tag is not allowed: .*javax\.script\.ScriptEngineManager"
(parse-string dangerous-yaml))
"by default, SnakeYaml stops creation of classes - malicious example")
(is (thrown-with-msg? YAMLException #"(?m).*could not.*constructor.*java\.lang\.Long"
(is (thrown-with-msg? YAMLException #"(?m).*Global tag is not allowed: .*java\.lang\.Long"
(parse-string "!!java.lang.Long 5"))
"by default, SnakeYaml stops creation of classes - innocuous looking class example"))

Expand Down

0 comments on commit 2799a9b

Please sign in to comment.