From 769ddd9d21d4fcb29e8e2a986a8847c4c1b59237 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Mon, 21 Sep 2026 22:54:52 +0900 Subject: [PATCH] Check return value of `yaml_*_event_initialize` in the emitter libyaml's `yaml_*_event_initialize()` functions reject an anchor, tag, or value that is not valid UTF-8 (and can fail on allocation errors), returning 0 and leaving the passed `yaml_event_t` untouched. The emitter ignored that return value and emitted the event anyway. It may cause unexpected behavior. For example, `yaml_emitter_emit()` may touch an invalid address or double free an invalid event. We can avoid it by checking every `yaml_*_event_initialize()` call and raise instead of emitting a stale or uninitialized event. --- ext/psych/psych_emitter.c | 45 +++++++++++++++++++++++++------------- test/psych/helper.rb | 7 ++++++ test/psych/test_emitter.rb | 39 +++++++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 15 deletions(-) diff --git a/ext/psych/psych_emitter.c b/ext/psych/psych_emitter.c index 187aebc3..c8823008 100644 --- a/ext/psych/psych_emitter.c +++ b/ext/psych/psych_emitter.c @@ -112,7 +112,8 @@ static VALUE start_stream(VALUE self, VALUE encoding) TypedData_Get_Struct(self, yaml_emitter_t, &psych_emitter_type, emitter); Check_Type(encoding, T_FIXNUM); - yaml_stream_start_event_initialize(&event, (yaml_encoding_t)NUM2INT(encoding)); + if(!yaml_stream_start_event_initialize(&event, (yaml_encoding_t)NUM2INT(encoding))) + rb_raise(rb_eRuntimeError, "failed to initialize stream start event"); emit(emitter, &event); @@ -131,7 +132,8 @@ static VALUE end_stream(VALUE self) yaml_event_t event; TypedData_Get_Struct(self, yaml_emitter_t, &psych_emitter_type, emitter); - yaml_stream_end_event_initialize(&event); + if(!yaml_stream_end_event_initialize(&event)) + rb_raise(rb_eRuntimeError, "failed to initialize stream end event"); emit(emitter, &event); @@ -207,13 +209,15 @@ static VALUE start_document_try(VALUE d) } } - yaml_document_start_event_initialize( + if(!yaml_document_start_event_initialize( &event, (RARRAY_LEN(version) > 0) ? &version_directive : NULL, data->head, tail, imp ? 1 : 0 - ); + )) { + rb_raise(rb_eRuntimeError, "failed to initialize document start event"); + } emit(emitter, &event); @@ -262,7 +266,8 @@ static VALUE end_document(VALUE self, VALUE imp) yaml_event_t event; TypedData_Get_Struct(self, yaml_emitter_t, &psych_emitter_type, emitter); - yaml_document_end_event_initialize(&event, imp ? 1 : 0); + if(!yaml_document_end_event_initialize(&event, imp ? 1 : 0)) + rb_raise(rb_eRuntimeError, "failed to initialize document end event"); emit(emitter, &event); @@ -307,7 +312,7 @@ static VALUE scalar( } const char *value_ptr = StringValuePtr(value); - yaml_scalar_event_initialize( + if(!yaml_scalar_event_initialize( &event, (yaml_char_t *)(NIL_P(anchor) ? NULL : StringValueCStr(anchor)), (yaml_char_t *)(NIL_P(tag) ? NULL : StringValueCStr(tag)), @@ -316,7 +321,9 @@ static VALUE scalar( plain ? 1 : 0, quoted ? 1 : 0, (yaml_scalar_style_t)NUM2INT(style) - ); + )) { + rb_raise(rb_eRuntimeError, "failed to initialize scalar event"); + } emit(emitter, &event); @@ -354,13 +361,15 @@ static VALUE start_sequence( TypedData_Get_Struct(self, yaml_emitter_t, &psych_emitter_type, emitter); - yaml_sequence_start_event_initialize( + if(!yaml_sequence_start_event_initialize( &event, (yaml_char_t *)(NIL_P(anchor) ? NULL : StringValueCStr(anchor)), (yaml_char_t *)(NIL_P(tag) ? NULL : StringValueCStr(tag)), implicit ? 1 : 0, (yaml_sequence_style_t)NUM2INT(style) - ); + )) { + rb_raise(rb_eRuntimeError, "failed to initialize sequence start event"); + } emit(emitter, &event); @@ -379,7 +388,8 @@ static VALUE end_sequence(VALUE self) yaml_event_t event; TypedData_Get_Struct(self, yaml_emitter_t, &psych_emitter_type, emitter); - yaml_sequence_end_event_initialize(&event); + if(!yaml_sequence_end_event_initialize(&event)) + rb_raise(rb_eRuntimeError, "failed to initialize sequence end event"); emit(emitter, &event); @@ -418,13 +428,15 @@ static VALUE start_mapping( tag = rb_str_export_to_enc(tag, encoding); } - yaml_mapping_start_event_initialize( + if(!yaml_mapping_start_event_initialize( &event, (yaml_char_t *)(NIL_P(anchor) ? NULL : StringValueCStr(anchor)), (yaml_char_t *)(NIL_P(tag) ? NULL : StringValueCStr(tag)), implicit ? 1 : 0, (yaml_mapping_style_t)NUM2INT(style) - ); + )) { + rb_raise(rb_eRuntimeError, "failed to initialize mapping start event"); + } emit(emitter, &event); @@ -443,7 +455,8 @@ static VALUE end_mapping(VALUE self) yaml_event_t event; TypedData_Get_Struct(self, yaml_emitter_t, &psych_emitter_type, emitter); - yaml_mapping_end_event_initialize(&event); + if(!yaml_mapping_end_event_initialize(&event)) + rb_raise(rb_eRuntimeError, "failed to initialize mapping end event"); emit(emitter, &event); @@ -467,10 +480,12 @@ static VALUE alias(VALUE self, VALUE anchor) anchor = rb_str_export_to_enc(anchor, rb_utf8_encoding()); } - yaml_alias_event_initialize( + if(!yaml_alias_event_initialize( &event, (yaml_char_t *)(NIL_P(anchor) ? NULL : StringValueCStr(anchor)) - ); + )) { + rb_raise(rb_eRuntimeError, "failed to initialize alias event"); + } emit(emitter, &event); diff --git a/test/psych/helper.rb b/test/psych/helper.rb index b6bf2013..91a34253 100644 --- a/test/psych/helper.rb +++ b/test/psych/helper.rb @@ -21,6 +21,13 @@ def libfyaml? defined?(Psych::BACKEND) && Psych::BACKEND == 'libfyaml' end + # True when psych uses the default libyaml C backend (not libfyaml and + # not the JRuby backend, both of which leave Psych::BACKEND either set to + # a different value or undefined). + def libyaml? + defined?(Psych::BACKEND) && Psych::BACKEND == 'libyaml' + end + def with_default_external(enc) verbose, $VERBOSE = $VERBOSE, nil origenc, Encoding.default_external = Encoding.default_external, enc diff --git a/test/psych/test_emitter.rb b/test/psych/test_emitter.rb index 7755fec0..b7894089 100644 --- a/test/psych/test_emitter.rb +++ b/test/psych/test_emitter.rb @@ -100,6 +100,45 @@ def test_start_sequence_arg_error end end + def test_invalid_event + omit 'libyaml backend only' unless libyaml? + + invalid = "\xFF" + + bad_calls = [ + ->(e) { e.scalar(invalid, nil, nil, false, true, 1) }, + ->(e) { e.scalar('x', invalid, nil, false, true, 1) }, + ->(e) { e.scalar('x', nil, invalid, false, true, 1) }, + ->(e) { e.start_sequence(invalid, nil, false, 1) }, + ->(e) { e.start_sequence(nil, invalid, false, 1) }, + ->(e) { e.start_mapping(invalid, nil, false, 1) }, + ->(e) { e.start_mapping(nil, invalid, false, 1) }, + ->(e) { e.alias(invalid) }, + ] + + bad_calls.each do |bad_call| + out = StringIO.new(''.dup) + emitter = Psych::Emitter.new out + emitter.start_stream Psych::Nodes::Stream::UTF8 + emitter.start_document [], [], true + # Emit a valid event that allocates memory (its anchor and tag). + emitter.start_sequence 'anchor', 'tag:example.com,2000:seq', false, 1 + + assert_raise(RuntimeError) { bad_call.call(emitter) } + end + end + + def test_invalid_tag_directive + omit 'libyaml backend only' unless libyaml? + + invalid = "\xFF" + + @emitter.start_stream Psych::Nodes::Stream::UTF8 + assert_raise(RuntimeError) do + @emitter.start_document [1, 1], [[invalid, 'tag:x']], false + end + end + def test_resizing_tags @emitter.start_stream Psych::Nodes::Stream::UTF8