-
Notifications
You must be signed in to change notification settings - Fork 318
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
Find heap buffer overflow by running fuzz test #258
Comments
…item, releated issue:yaml#258
It would be really good to find out how to reproduce. Looking at the stacktrace
So in I added this code in
and it never prints EMPTY. |
I tried to reproduce it by following those instructions: https://google.github.io/oss-fuzz/advanced-topics/reproducing/#building-using-docker
|
Also note that I haven't been able to run
|
Apparently there are people who can reproduce it. |
Yes, I think it is uncommon to publish the exact attack input. |
I was able to reproduce it now with the help of a colleague. Apparently podman is doing something wrong. With docker it worked and built all the fuzzers. The problem only appears when the emitter writes to a buffer:
"fixes" it. #259 makes the symptom go away, but the function shouldn't be called in this case at all. So I assume the memory is corrupted even before and #259 would only hide a symptom. |
I should also note that the write_handler is only called once, after the 132 sequence end events, and when I look into the buffer content at that point, it is missing the last closing |
you mean that this is a problem with the oss-fuzz test method? |
no, I was just blindly guessing. but I found out that it starts to go wrong even before the write handler is called, in one of the |
It's confusing, and I don't think this is worth applying for a cve number. |
Is it possible to remove the cve number? Or reject it, After all the code doesn't seem to be going here. |
in addition, even if this is an external issue, I think the pr is ok The macro does not do any out-of-bounds checking, so this is needed, or else do the out-of-bounds checking in the macro.
|
It's true that it might be good to check before the POP. but it should return an error in this case because the code should not run into this state, and the resulting YAML will be invalid. |
see #290 for a mitigation. It will still output invalid yaml because it's missing the last |
Can you provide a way to reproduce it? |
The reproducer can be found in the CVE. you have to use the oss fuzzer. |
so after the mitigation, can I assume there is still a bug considering the invalid yaml? After reproducing it, I found this text: |
@znzjugod What do you mean with "I found this text"? Is this something that was emitted by libyaml? |
This text is generated by the oss-fuzz and stored when the stack overflow occurs. I understand that this text is from the oss-fuzz. When an error occurs, the data generated by oss-fuzz is automatically saved in crash-xxxxxxxxxx. |
I think this content should be this
|
i open another issue to tracing failure of |
is there any update news? |
I had a closer look at the code again. So far I was not familiar with that part of libyaml. 1. The write handler returns
|
I opened an issue for the fuzzer now: google/oss-fuzz#11811 |
I contacted cve.mitre.org and nvd.nist.gov to reject the CVE and still waiting for a reply. |
I'm wondering if there has been any update on this? I saw that the fuzzer was at least partially updated more than a month ago, but no further updates since then. This issue has caused pyyaml to be flagged and we're stuck in limbo at the moment. |
I was unsuccessful with contacting CVE authorities so far and I will try another contact method. |
Thanks a lot. I appreciate it. |
The CVE is now rejected: https://www.cve.org/CVERecord?id=CVE-2024-3205 |
hi, I am using the oss-fuzz google/oss-fuzz against libyaml and when I run libyaml_dumper_fuzzer I find a heap buffer overflow error in function: yaml_emitter_emit_flow_sequence_item.
the erro log is in this pic:
I also attach the full error log here:
fuzz_error_log.TXT
Due to my limited knowledge of fuzz test I don't know how to find the exact input yaml or string to reproduce this error, but I think the error log can help us to analysis and fix the error.
Code analysis:
It is obvious that in emitter.c line 761, we try to pop the element from STACK and get emitter->indents value. However, we didn't check whether STACK is empty and in this case, we try to dereference a pointer: (*(--(stack).top)) and stack.top maybe NULL and cause heap buffer overflow.
Fix:
I think it's necessary to add STACK_EMPTY before POP, the goal is to check whether stack.top has values and avoid dereferencing a NULL pointer.
I will create a PR to fix this problem. #259
The text was updated successfully, but these errors were encountered: