-
Notifications
You must be signed in to change notification settings - Fork 346
Description
The following CVE is a misuse of the libyaml API:
- https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2024-35325 https://github.com/idhyt/pocs/blob/main/libyaml/CVE-2024-35325.c
I can copy the relevant part of the code:
void poc() {
yaml_document_t document;
memset(&document, 0, sizeof(yaml_document_t));
yaml_document_initialize(&document, NULL, NULL, NULL, 0, 0);
yaml_event_t event;
memset(&event, 0, sizeof(yaml_event_t));
int encoding = YAML_ANY_ENCODING;
yaml_document_add_sequence(&document, YAML_NULL_TAG, YAML_ANY_SEQUENCE_STYLE);
// step1: allocated by yaml_strdup(anchor) at api.c:887
yaml_sequence_start_event_initialize(&event, "anchor", YAML_NULL_TAG, 0, YAML_ANY_SEQUENCE_STYLE);
yaml_emitter_t emitter;
memset(&emitter, 0, sizeof(yaml_emitter_t));
yaml_emitter_initialize(&emitter);
// step2: yaml_emitter_emit call ENQUEUE (emitter.c:288) copy data from event to emitter.events.tail -> (*((queue).tail++) = value, 1)
yaml_emitter_emit(&emitter, &event);
// step3: first free at api.c:400 -> yaml_event_delete(&DEQUEUE(emitter, emitter->events));
yaml_emitter_delete(&emitter);
// step4: double free at api.c:1015
yaml_event_delete(&event);
yaml_document_delete(&document);
}
yaml_emitter_emit()
is responsible for deleting the events, you are not supposed to call yaml_event_delete()
yourself.
yaml_event_delete()
cannot check if the event was already deleted due to the nature of the struct. The struct would have to be changed, and all code using libyaml, for example bindings, would have to be changed.
I couldn't find a way to check it wit the current struct.
The vulnerability is in code that is using libyaml in a wrong way, not in libyaml directly.
Of course nowadays one might say the design of libyaml is bad and should prevent such misusage, but libyaml is quite a few years old, and preventing that will break things, like I said, and would be quite some work, and I don't know anyone who would have the free time for this.
So I'm not sure if that counts as a CVE.
I can improve the documentation (when I have some free time).
Anyone who knows more about when something is deserving to have a CVE or not is welcome to comment.
There was already a discussion about that CVE in #297 but the thread is distracting because I was arguing with the issue author about the way it was reported and published.