Skip to content

Commit 7799fbd

Browse files
authored
fix: avoid NPE on uninitialized XML::Node structs (v1.19.x) (#3645)
**What problem is this PR intended to solve?** [CRuby] Fixed a null pointer dereference when methods are called on uninitialized wrapper objects (e.g. via `allocate`); these now raise instead of crashing the process. See [GHSA-9cv2-cfxc-v4v2](GHSA-9cv2-cfxc-v4v2) for more information. **Have you included adequate test coverage?** Yes. **Does this change affect the behavior of either the C or the Java implementations?** The vulnerability affects CRuby only. The equivalent guard was added to the Java implementation for parity; both now raise rather than crash.
2 parents ef19e13 + e63c52c commit 7799fbd

6 files changed

Lines changed: 36 additions & 5 deletions

File tree

ext/java/nokogiri/XmlAttr.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,9 @@ public class XmlAttr extends XmlNode
100100
public IRubyObject
101101
value_set(ThreadContext context, IRubyObject content)
102102
{
103+
if (node == null) {
104+
throw context.runtime.newRuntimeError("Uninitialized " + getMetaClass().getRealClass().getName() + " struct (null data pointer)");
105+
}
103106
Attr attr = (Attr) node;
104107
if (content != null && !content.isNil()) {
105108
attr.setValue(rubyStringToString(XmlNode.encode_special_chars(context, content)));

ext/nokogiri/nokogiri.h

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,8 +181,17 @@ int noko_io_read(void *ctx, char *buffer, int len);
181181
int noko_io_write(void *ctx, char *buffer, int len);
182182
int noko_io_close(void *ctx);
183183

184-
#define Noko_Node_Get_Struct(obj,type,sval) ((sval) = (type*)DATA_PTR(obj))
185-
#define Noko_Namespace_Get_Struct(obj,type,sval) ((sval) = (type*)DATA_PTR(obj))
184+
static inline void *
185+
_noko_data_ptr(VALUE rb_object)
186+
{
187+
void *c_data = DATA_PTR(rb_object);
188+
if (c_data == NULL) {
189+
rb_raise(rb_eRuntimeError, "Uninitialized %" PRIsVALUE " struct (null data pointer)", rb_obj_class(rb_object));
190+
}
191+
return c_data;
192+
}
193+
#define Noko_Node_Get_Struct(obj,type,sval) ((sval) = (type*)_noko_data_ptr(obj))
194+
#define Noko_Namespace_Get_Struct(obj,type,sval) ((sval) = (type*)_noko_data_ptr(obj))
186195

187196
VALUE noko_xml_node_wrap(VALUE klass, xmlNodePtr node) ;
188197
VALUE noko_xml_node_wrap_node_set_result(xmlNodePtr node, VALUE node_set) ;

ext/nokogiri/xml_document.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -712,6 +712,9 @@ noko_xml_document_unwrap(VALUE rb_document)
712712
{
713713
xmlDocPtr c_document;
714714
TypedData_Get_Struct(rb_document, xmlDoc, &xml_doc_type, c_document);
715+
if (c_document == NULL) {
716+
rb_raise(rb_eRuntimeError, "Uninitialized %" PRIsVALUE " struct (null data pointer)", rb_obj_class(rb_document));
717+
}
715718
return c_document;
716719
}
717720

ext/nokogiri/xml_node.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2351,9 +2351,7 @@ in_context(VALUE self, VALUE _str, VALUE _options)
23512351
VALUE
23522352
rb_xml_node_data_ptr_eh(VALUE self)
23532353
{
2354-
xmlNodePtr c_node;
2355-
Noko_Node_Get_Struct(self, xmlNode, c_node);
2356-
return c_node ? Qtrue : Qfalse;
2354+
return DATA_PTR(self) ? Qtrue : Qfalse;
23572355
}
23582356

23592357
VALUE

test/xml/test_attr.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,14 @@ def test_new_raises_argerror_on_nondocument
1313
end
1414
end
1515

16+
def test_value_set_on_uninitialized_attr_raises_without_crashing
17+
attr = Nokogiri::XML::Attr.allocate
18+
19+
refute_valgrind_errors(yield_on_jruby: true) do
20+
assert_raises(RuntimeError) { attr.value = "x" }
21+
end
22+
end
23+
1624
def test_content=
1725
xml = Nokogiri::XML.parse(File.read(XML_FILE), XML_FILE)
1826
address = xml.xpath("//address")[3]

test/xml/test_document.rb

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -651,6 +651,16 @@ def test_encoding
651651
assert_equal("UTF-8", xml.encoding)
652652
end
653653

654+
def test_encoding_set_on_uninitialized_document_raises_without_crashing
655+
skip_unless_libxml2("on jruby a bare document has some partial functionality")
656+
657+
doc = Nokogiri::XML::Document.allocate
658+
659+
refute_valgrind_errors do
660+
assert_raises(RuntimeError) { doc.encoding = "UTF-8" }
661+
end
662+
end
663+
654664
def test_memory_explosion_on_invalid_xml
655665
doc = Nokogiri::XML("<<<")
656666
refute_nil(doc)

0 commit comments

Comments
 (0)