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.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.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.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.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.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.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.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.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.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.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.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.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.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.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.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.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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants