Refuse blessed objects and code in YAML request bodies

[bigpresh]

What

Dancer2::Serializer::YAML::deserialize handed request bodies straight to
YAML::Load. A body tagged !!perl/hash:Some::Class therefore instantiated an
arbitrary blessed object — the entry point for DESTROY/AUTOLOAD/overload
gadget chains — and !!perl/code could ask for a string eval.

deserialize now sets $YAML::LoadBlessed = 0 and $YAML::LoadCode = 0 itself
(localised) before loading, rather than relying on YAML.pm's ambient defaults,
and the minimum YAML is raised to 1.30.

Why do it in our code and bump the dependency

• $YAML::LoadBlessed only exists from YAML 1.25; below that, setting it is a
  silent no-op. So the explicit setting needs a version floor to mean anything.
• Setting it ourselves (rather than leaning on 1.30's safe default) keeps the
  guarantee independent of which YAML the user resolved, and holds even when the
  surrounding process has set the variable to something hostile — which is not
  hypothetical: Dancer2::Session::YAML sets LoadBlessed = 1 for its own load.
• The floor is 1.30 rather than 1.25 because 1.26 fixed a parsing regression
  in 1.25 and 1.28 carries an upstream security fix in the same load path
  ("only enable loading globs when $LoadCode is set") that is behavioural and
  cannot be reproduced from our side. 1.30 only adds the changed default, which
  we override either way, but it is from Jan 2020 and costs nothing. The new
  floor supersedes the old 0.86 minimum and the 1.16 exclusion, now dropped.

Reachability

Not limited to apps that set serializer: YAML. Serializer::Mutable maps both
text/x-yaml and text/html to this class, so a request body under a
common content type reaches YAML::Load with no YAML configuration anywhere.
And Request deserializes the body eagerly, before any route runs, so a
DESTROY-based gadget does not even need the route to touch the parsed data.

The !!perl/code direct-eval path was gated by $YAML::LoadCode, which defaults
off and which Dancer2 never enabled, so it was not reachable by default; it is
closed here regardless as belt-and-braces.

Tests

t/integration/serializer/yaml.t sets a hostile ambient $YAML::LoadBlessed = 1
before each assertion, so it exercises the guard rather than passing on the safe
default a recent YAML would give anyway, and includes a control proving the
payload really does bless without it. The assertions test only the security
property ("no attacker-named object comes back"), not the exact shape of what
does — that shape is YAML-version-dependent (older YAML strips the tag and
returns a plain hash; newer YAML rejects the document), and pinning either one
is what made an earlier draft pass locally but fail on CI.

Full suite passes locally (208 files, 1851 tests) on YAML 1.30.

Related

The version split above surfaced #1837 (Role::Serializer's error handler is
not class-safe): on newer YAML a rejected blessed body throws, and a
class-method deserialize call routes that through the unguarded log_cb. The
framework always calls deserialize on an instance, so it is unaffected in
normal use, and the test now does the same — but #1837 is worth fixing on its
own.

[cromedome]

Nice catch! 👍 Approved.

[veryrusty]

👍 secure be default is the way to go. Thanks @bigpresh !

Note: I pondered the potential to break existing code that relied on insecure deserialisation; but settled on that its trivial to write your own serialiser if you want to do unsafe things.