Use safe YAML loading #493
Conversation
the latest release in this line. Does *not* by itself fix CVE-2022-1471. Signed-off-by: Matthias Rampke <[email protected]>
We don't *need* unsafe YAML deserialization, so make sure we don't. It is unlikely but not unthinkable that someone might load an untrusted configuration. Signed-off-by: Matthias Rampke <[email protected]>
|
@or-shachar could you take a look at this to make sure I did it right? |
There was a problem hiding this comment.
SafeConstructor indeed sounds safer :)
Important to note that this limits the kind of objects this constructor can return to standard java objects:
(from docs)
Also this helped me understand the motivation a little further.
The CloudWatchCollectorTest covers parsing yamls and loading configuration so I'm guessing this doesn't break anything.
Looks good to me :-)
future note:
Maybe we should explore defining a schema to the configuration and use type safe loading. I think this can be a good opportunity to use Java record sede. The gains would be a well defined configuration class with fields and more proper access to configuration fields.
Also - moving to configuration schema can help with #484.
|
I agree, a proper configuration schema would be much better: #512 |
We don't need unsafe YAML deserialization, so make sure we don't. It
is unlikely but not unthinkable that someone might load an untrusted
configuration.