dtolnay / serde-yaml

Strongly typed YAML library for Rust
Apache License 2.0
964 stars 164 forks source link

unsafe-yaml-backed serde-yaml panics on lexer errors #293

Closed Mrmaxmeier closed 2 years ago

Mrmaxmeier commented 2 years ago

YAML payloads with invalid syntax can trigger this branch: sys::YAML_NO_EVENT => unreachable!():

let _ = serde_yaml::from_str::<serde_yaml::Value>(">\n@");
thread 'main' panicked at 'internal error: entered unreachable code', src/libyaml/parser.rs:142:31
stack backtrace:
   0: rust_begin_unwind
             [...]
   3: serde_yaml::libyaml::parser::convert_event
             at ./src/libyaml/parser.rs:142:31
   4: serde_yaml::libyaml::parser::Parser::next
             at ./src/libyaml/parser.rs:91:23

This happens because libyaml's yaml_parser_parse seems to expect the caller to handle parser->error states: https://github.com/dtolnay/unsafe-libyaml/blob/5d421b89e946d70b52a1b8cf026d97244e182f39/src/parser.rs#L75

This patch fixes the panic, but I'm not sure if it's the proper fix:

diff --git a/src/libyaml/parser.rs b/src/libyaml/parser.rs
index 9f43ced..ae8f375 100644
--- a/src/libyaml/parser.rs
+++ b/src/libyaml/parser.rs
@@ -84,6 +84,9 @@ impl<'input> Parser<'input> {
         let mut event = MaybeUninit::<sys::yaml_event_t>::uninit();
         unsafe {
             let parser = addr_of_mut!((*self.pin.ptr).sys);
+            if (*parser).error != sys::YAML_NO_ERROR {
+                return Err(Error::parse_error(parser));
+            }
             let event = event.as_mut_ptr();
             if sys::yaml_parser_parse(parser, event).fail {
                 return Err(Error::parse_error(parser));
dtolnay commented 2 years ago

Thanks! Fixed in 0.9.1.