-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Problems in libyaml fuzzer programs #11811
Comments
This was referenced Apr 16, 2024
perlpunk
added a commit
to perlpunk/oss-fuzz
that referenced
this issue
Apr 18, 2024
See google#11811 libyaml expects 1 for success and 0 for failure. https://github.com/yaml/libyaml/blob/master/src/writer.c#L53-L62 if (emitter->write_handler(emitter->write_handler_data, ...) { ... } else { return yaml_emitter_set_writer_error(emitter, "write error"); } The logic is currently reverse, which (in combination of the failing check for the yaml_emitter_dump return value) caused several wrong bug reports and a CVE. The fuzzer programs just ignore the failing yaml_emitter_dump, and so I assume it never appeared as a problem. Only in the cases where the wrongly called yaml_emitter_close ran into a case where it popped from an empty stack an overflow was detected. The input YAML in question just had a lot of nested sequences in the form - - - - - which in canonical output mode resulted in a large output because of the indentation, and so the buffer flush was triggered before the emitter finished: !!seq [ !!seq [ ... In the most cases the YAML is simply too small to produce the error because the flush happened when the output was complete.
I created a PR to fix one of the problems: |
DonggeLiu
pushed a commit
that referenced
this issue
Apr 19, 2024
See #11811 libyaml expects 1 for success and 0 for failure. https://github.com/yaml/libyaml/blob/master/src/writer.c#L53-L62 if (emitter->write_handler(emitter->write_handler_data, ...) { ... } else { return yaml_emitter_set_writer_error(emitter, "write error"); } The logic is currently reverse, which (in combination of the failing check for the yaml_emitter_dump return value) caused several wrong bug reports and a CVE. The fuzzer programs just ignore the failing yaml_emitter_dump, and so I assume it never appeared as a problem. Only in the cases where the wrongly called yaml_emitter_close ran into a case where it popped from an empty stack an overflow was detected. The input YAML in question just had a lot of nested sequences in the form - - - - - which in canonical output mode resulted in a large output because of the indentation, and so the buffer flush was triggered before the emitter finished: !!seq [ !!seq [ ... In the most cases the YAML is simply too small to produce the error because the flush happened when the output was complete. Note that this does not yet fix the missing error handling of `yaml_emitter_dump` in libyaml_dumper_fuzzer.c etc.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This is about https://nvd.nist.gov/vuln/detail/CVE-2024-3205
When trying to reproduce it, I wasn't able to get the same behaviour when using a normal buffer. I only saw it when using the fuzzer code.
I think there are two mistakes in the libyaml fuzzers.
One only applies to the libyaml_dumper_fuzzer, the other is about the write_handler used in the other programs as well.
Please see my detailed explanation in yaml/libyaml#258 (comment)
Because of this, we are rejecting the CVE.
I would make a PR but am not sure about the expected behaviour.
The text was updated successfully, but these errors were encountered: