Skip to content
This repository has been archived by the owner on Mar 25, 2024. It is now read-only.

Out of bounds access in output generated by PyYAML #68

Closed
dtolnay opened this issue May 30, 2017 · 9 comments
Closed

Out of bounds access in output generated by PyYAML #68

dtolnay opened this issue May 30, 2017 · 9 comments

Comments

@dtolnay
Copy link
Owner

dtolnay commented May 30, 2017

Moved from #49 (comment).


Hi, I ran into the Out of bounds access in the wild with some output generated by PyYAML. I minimized it to a pretty simple example:

---
content: "Foo\
    Bar"

Is this just a case of missing support for line continuations? Should I file a separate ticket for that?


@radix

@dtolnay
Copy link
Owner Author

dtolnay commented May 30, 2017

I was not able to reproduce this. Are you sure the out of bounds wasn't in your code?

extern crate yaml_rust;
extern crate serde_yaml;

fn main() {
    let y = r#"---
content: "Foo\
    Bar""#;

    println!("{}", y);
    println!("{:?}", yaml_rust::YamlLoader::load_from_str(y).unwrap());
    println!("{:?}", serde_yaml::from_str::<serde_yaml::Value>(y).unwrap());
}

@radix
Copy link

radix commented May 30, 2017

I'll work on writing a minimal script that reproduces it. So far I assumed it's the library because the backtrace goes through yaml_rust (which I now remember is a separate crate... maybe this issue should be moved). Here's the stack trace in the mean time.

thread 'main' panicked at 'Out of bounds access', src\libcore\option.rs:823
stack backtrace:
   0: std::sys_common::backtrace::_print
             at C:\projects\rust\src\libstd\sys_common\backtrace.rs:94
   1: std::panicking::default_hook::{{closure}}
             at C:\projects\rust\src\libstd\panicking.rs:354
   2: std::panicking::default_hook
             at C:\projects\rust\src\libstd\panicking.rs:371
   3: std::panicking::rust_panic_with_hook
             at C:\projects\rust\src\libstd\panicking.rs:549
   4: std::panicking::begin_panic<collections::string::String>
             at C:\projects\rust\src\libstd\panicking.rs:511
   5: std::panicking::begin_panic_fmt
             at C:\projects\rust\src\libstd\panicking.rs:495
   6: std::panicking::rust_begin_panic
             at C:\projects\rust\src\libstd\panicking.rs:471
   7: core::panicking::panic_fmt
             at C:\projects\rust\src\libcore\panicking.rs:69
   8: core::option::expect_failed
             at C:\projects\rust\src\libcore\option.rs:823
   9: core::option::Option<&char>::expect<&char>
             at C:\projects\rust\src\libcore\option.rs:302
  10: collections::vec_deque::{{impl}}::index<char>
             at C:\projects\rust\src\libcollections\vec_deque.rs:2347
  11: yaml_rust::scanner::Scanner<core::str::Chars>::ch<core::str::Chars>
             at C:\Users\radix\.cargo\registry\src\github.heygears.com-1ecc6299db9ec823\yaml-rust-0.3.5\src\scanner.rs:268
  12: yaml_rust::scanner::Scanner<core::str::Chars>::scan_flow_scalar<core::str::Chars>
             at C:\Users\radix\.cargo\registry\src\github.heygears.com-1ecc6299db9ec823\yaml-rust-0.3.5\src\scanner.rs:1297
  13: yaml_rust::scanner::Scanner<core::str::Chars>::fetch_flow_scalar<core::str::Chars>
             at C:\Users\radix\.cargo\registry\src\github.heygears.com-1ecc6299db9ec823\yaml-rust-0.3.5\src\scanner.rs:1171
  14: yaml_rust::scanner::Scanner<core::str::Chars>::fetch_next_token<core::str::Chars>
             at C:\Users\radix\.cargo\registry\src\github.heygears.com-1ecc6299db9ec823\yaml-rust-0.3.5\src\scanner.rs:385
  15: yaml_rust::scanner::Scanner<core::str::Chars>::fetch_more_tokens<core::str::Chars>
             at C:\Users\radix\.cargo\registry\src\github.heygears.com-1ecc6299db9ec823\yaml-rust-0.3.5\src\scanner.rs:430
  16: yaml_rust::scanner::Scanner<core::str::Chars>::next_token<core::str::Chars>
             at C:\Users\radix\.cargo\registry\src\github.heygears.com-1ecc6299db9ec823\yaml-rust-0.3.5\src\scanner.rs:401
  17: yaml_rust::scanner::{{impl}}::next<core::str::Chars>
             at C:\Users\radix\.cargo\registry\src\github.heygears.com-1ecc6299db9ec823\yaml-rust-0.3.5\src\scanner.rs:147
  18: yaml_rust::parser::Parser<core::str::Chars>::peek<core::str::Chars>
             at C:\Users\radix\.cargo\registry\src\github.heygears.com-1ecc6299db9ec823\yaml-rust-0.3.5\src\parser.rs:114
  19: yaml_rust::parser::Parser<core::str::Chars>::block_mapping_value<core::str::Chars>
             at C:\Users\radix\.cargo\registry\src\github.heygears.com-1ecc6299db9ec823\yaml-rust-0.3.5\src\parser.rs:524
  20: yaml_rust::parser::Parser<core::str::Chars>::state_machine<core::str::Chars>
             at C:\Users\radix\.cargo\registry\src\github.heygears.com-1ecc6299db9ec823\yaml-rust-0.3.5\src\parser.rs:255
  21: yaml_rust::parser::Parser<core::str::Chars>::parse<core::str::Chars,serde_yaml::de::Loader>
             at C:\Users\radix\.cargo\registry\src\github.heygears.com-1ecc6299db9ec823\yaml-rust-0.3.5\src\parser.rs:144
  22: yaml_rust::parser::Parser<core::str::Chars>::load_mapping<core::str::Chars,serde_yaml::de::Loader>
             at C:\Users\radix\.cargo\registry\src\github.heygears.com-1ecc6299db9ec823\yaml-rust-0.3.5\src\parser.rs:217
  23: yaml_rust::parser::Parser<core::str::Chars>::load_node<core::str::Chars,serde_yaml::de::Loader>
             at C:\Users\radix\.cargo\registry\src\github.heygears.com-1ecc6299db9ec823\yaml-rust-0.3.5\src\parser.rs:202
  24: yaml_rust::parser::Parser<core::str::Chars>::load_document<core::str::Chars,serde_yaml::de::Loader>
             at C:\Users\radix\.cargo\registry\src\github.heygears.com-1ecc6299db9ec823\yaml-rust-0.3.5\src\parser.rs:183
  25: yaml_rust::parser::Parser<core::str::Chars>::load<core::str::Chars,serde_yaml::de::Loader>
             at C:\Users\radix\.cargo\registry\src\github.heygears.com-1ecc6299db9ec823\yaml-rust-0.3.5\src\parser.rs:170
  26: serde_yaml::de::from_str<pandt::types::App>
             at C:\Users\radix\.cargo\registry\src\github.heygears.com-1ecc6299db9ec823\serde_yaml-0.7.0\src\de.rs:636
  27: ptrpi::load_app_from_path
             at .\src\main.rs:212
  28: ptrpi::main
             at .\src\main.rs:228
  29: panic_unwind::__rust_maybe_catch_panic
             at C:\projects\rust\src\libpanic_unwind\lib.rs:98
  30: std::rt::lang_start
             at C:\projects\rust\src\libstd\rt.rs:52
  31: main
  32: __scrt_common_main_seh
             at f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:253
  33: BaseThreadInitThunk

@dtolnay
Copy link
Owner Author

dtolnay commented May 30, 2017

That definitely looks like a bug. If you are able to reproduce it with a short self-contained example, please open a yaml-rust issue.

@radix
Copy link

radix commented May 30, 2017

For some reason the code that has the static string won't reproduce it for me, but I wrote a script that loads it from a file and it does break in the same way.

Also I notice that SenseTime-Cloud committed a fix to yaml_rust that fixes an Out of bounds that hasn't yet been released. I'm going to try with yaml_rust master first and see if that fixes it, and if not I'll file a bug on yaml-rust.

@radix
Copy link

radix commented May 30, 2017

Yep, that was it. After updating to yaml-rust git, it's fixed.

I'm still not sure why your script didn't reproduce it, so here's mine for posterity:

extern crate serde_yaml;
extern crate yaml_rust;

use std::env;
use std::fs::File;
use std::io::Read;

fn main() {
  let filename = "buggy-game.yaml";
  let mut appf = File::open(filename).unwrap();
  let mut apps = String::new();
  appf.read_to_string(&mut apps).unwrap();
  println!("YAML_RUST");
  println!("{:?}", yaml_rust::YamlLoader::load_from_str(&apps).unwrap());
  let x: () = serde_yaml::from_str(&apps).unwrap();
  println!("SERDE_YAML");
  println!("{:?}", x);
}

buggy-game.yaml:

---
content: "Foo\
    Bar"

After updating yaml-rust to master, it prints it out successfully but the serde_yaml breaks (because i guess it's pinned to yaml-rust 0.3.5).

@dtolnay
Copy link
Owner Author

dtolnay commented May 30, 2017

That looks like a different issue. Serde won't let you deserialize () from buggy-game.yaml because () corresponds to null or ~ in yaml which is not what the file contains. Try this:

- let x: () = serde_yaml::from_str(&apps).unwrap();
+ let x: serde_yaml::Value = serde_yaml::from_str(&apps).unwrap();

@dtolnay
Copy link
Owner Author

dtolnay commented May 30, 2017

In any case I still can't reproduce this even with the File read_to_string code and yaml-rust 0.3.5, so the fix in yaml-rust master may not be the issue you hit.

YAML_RUST
[Hash({String("content"): String("FooBar")})]
SERDE_YAML
Mapping(Mapping { map: {String("content"): String("FooBar")} })

@radix
Copy link

radix commented May 30, 2017

Figured it out. I'm on windows, my file has \r\n, not just \n, and it's definitely fixed by the change in yaml-rust.

About the () vs Value, I tried that change and it didn't change the behavior. Normally if it can't make the content fit the type it gives a different error than Out of bounds.

@dtolnay
Copy link
Owner Author

dtolnay commented May 30, 2017

Great, thanks for confirming that it is fixed! I will need to cut a yaml-rust release when I get a chance.

For me with \n newlines, the error was "invalid type: map, expected unit".

@dtolnay dtolnay closed this as completed May 30, 2017
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants