Clojurians Log v2
clj-yaml
I thought this was a bug in clj-yaml:
user=> (-> {"a b" "c d"} clj-yaml.core/generate-string clj-yaml.core/parse-string)
{:a b "c d"}
user=> {:a b "c d"}
Syntax error reading source at (REPL:1:13).
Map literal must contain an even number of forms
Awake again. So in your example, you are explicitly calling eval. There's nothing a library can do to stop folks from calling eval on things.
Well sure. I was just coming up with a quick example of how that yaml load could lead to something completely unexpected and possibly exploitable.
I think the main point here is that it's probably a bad idea to load all scalar keys into keywords.
But if you already have something the user can do to change that then I guess it's okay for now.
Agreed, we would not have made that choice a default, from the user guide:
> The :keywords option defaults to true for historical reasons.
It's a default behavior we inherited.
Ah. Got it.
Hmmm... yeah, a keyword with spaces won't do to well with round-tripping:
user=> (keyword "a b")
:a b
user=> (-> "a b" keyword pr-str)
":a b"
user=> (-> "a b" keyword pr-str read-string)
:a:keywords false on parse.user=> (-> {"a b" "c d"} clj-yaml.core/generate-string clj-yaml.core/parse-string clj-yaml.core/generate-string clj-yaml.core/parse-string)
{:a b "c d"}I think it could likely be considered a bug in pr-str
it could make (keyword "a b") instead I should think
maybe it's not a good idea for clj-yaml to load keys with spaces or other weird things as keywords
The annoyance is that keyword can generate Clojure keywords that are technically invalid that can be printed but not read in.
Questions around this come up often here on Slack.
See keywords under https://clojure.org/reference/reader#_literals
> maybe it's not a good idea for clj-yaml to load keys with spaces or other weird things as keywords Huh, we documented this https://github.com/clj-commons/clj-yaml/blob/master/doc/01-user-guide.adoc#key-conversion: > When clj-yaml detects a key that cannot be converted to a Clojure keyword, it will leave it unconverted. Detection and conversion is not sophisticated and can result in keywords that are illegal and unreadable.
We could bump up the illegal keyword detection if this is making folks suffer.
I'm imagining a code exploit here...
user=> (-> {"x (println 123)" 1 "y (println 456)" 2} clj-yaml.core/generate-string clj-yaml.core/parse-string str read-string eval)
123
456
{:x nil, 1 :y, nil 2}Ya, but you probably don't want to eval from untrusted data, yeah? And you'd want to use clojure.edn/read-string as well.
I'm not saying I'd write that code. I'm saying that people might find a way to use it in an exploitable way
Gotta hit the hay for today!
o/
this reminds me of pyyaml documenting:
Loading YAML
Warning: It is not safe to call yaml.load with any data received from an untrusted source! yaml.load is as powerful as pickle.load and so may call any Python function. Check the yaml.safe_load function though.yaml.load needed to call yaml.safe_loadI'll re-read your example tomorrow, I don't think I even read it right.... 😴
ok. have a good sleep
Yeah there is a SnakeYAML SafeConstructor we use by default. So we don't instantiate Java classes without the user opting in. But 💤
user=> (require 'clj-yaml.core) (def v [1 2 3]) (-> [v [1 2 3] v] clj-yaml.core/generate-string print)
nil
#'user/v
- [1, 2, 3]
- [1, 2, 3]
- [1, 2, 3]
nil
user=> $ perl -MYAML::PP -e '$v = [1, 2, 3]; print YAML::PP::Dump [$v, [1, 2, 3], $v]'
---
- &1
- 1
- 2
- 3
- - 1
- 2
- 3
- *1$ python -c 'import yaml; v = [1, 2, 3]; print(yaml.dump([v, [1, 2, 3], v]))'
- &id001
- 1
- 2
- 3
- - 1
- 2
- 3
- *id001Thanks for the input @ingy! Clj-yaml has many rough edges but is hopefully good enough for most folks to parse/generate the odd bit of YAML. Do you think clj-yaml needs to alias identical values? What problem does this solve for clj-yaml users? Can we just wait for some hypothetical future reference implementation to handle this type of thing? simple_smile
Yep sounds good.
I do think this is probably a fairly common case if people are using YAML as a data serialization language which is what it was originally made for.
And I don't think you can create cyclic data structures with Clojure immutable values. So that simply is not a use case.
But if nobody's clamoring for it it's probably up to me to make a PR 😄
And if nobody is clamoring, maybe you shouldn't create that PR!
For cyclic structures, maybe a strategy like https://github.com/aroemers/rmap could work. Worth maybe exploring. But nobody is clamoring for this for clj-yaml. So not going to look into it at this time.