Crash in XMLParser::PImpl::loadDocImpl when processing malformed XML due to NULL-ptr dereference

[cktii]

Description

A null pointer dereference in XMLParser::PImpl::loadDocImpl can lead to a crash (illegal instruction) when processing malformed XML. The crash occurs because the code assumes doc->RootElement() returns a valid pointer without checking:

BehaviorTree.CPP/src/xml_parsing.cpp

Lines 235 to 247 in 48f6c5b

	void XMLParser::PImpl::loadDocImpl(XMLDocument* doc, bool add_includes)
	{
	if(doc->Error())
	{
	char buffer[512];
	snprintf(buffer, sizeof buffer, "Error parsing the XML: %s", doc->ErrorStr());
	throw RuntimeError(buffer);
	}
	const XMLElement* xml_root = doc->RootElement();
	auto format = xml_root->Attribute("BTCPP_format");
	if(!format)

Found in commit: 48f6c5b

Bug Class

Memory Safety - Null Pointer Dereference

Backtrace

pwndbg> bt
#0  0x0000555555f3161f in BT::XMLParser::PImpl::loadDocImpl (this=0x50f0000027a0, doc=0x518000000080, add_includes=true)
    at /home/user/git/work/BehaviorTree.CPP/src/xml_parsing.cpp:246
#1  0x0000555555f349e3 in BT::XMLParser::loadFromText (this=0x7ffff5410260,
    xml_text="cS/></Sequence>querseqjckZat><ChjckBattB\236ttery\301enCe", '\272' <repeats 23 times>, "ttot_\\\\\"attery name=\"battery_\210k\"/></Secroot_s", add_includes=true) at /home/user/git/work/BehaviorTree.CPP/src/xml_parsing.cpp:178
#2  0x00005555558e9648 in BT::BehaviorTreeFactory::createTreeFromText (this=0x7ffff5709040,
    text="cS/></Sequence>querseqjckZat><ChjckBattB\236ttery\301enCe", '\272' <repeats 23 times>, "ttot_\\\\\"attery name=\"battery_\210k\"/></Secroot_s", blackboard=std::shared_ptr<BT::Blackboard> (use count 1, weak count 0) = {...})
    at /home/user/git/work/BehaviorTree.CPP/src/bt_factory.cpp:407

Root cause

A file with malformed XML that starts with a closing tag, e.g.:

cS/></Sequence>querseqjckZat><ChjckBattB

The crash occurs because:

When parsing malformed XML, doc->RootElement() may return nullptr. The code doesn't check for nullptr before dereferencing the pointer. Attempting to call Attribute() on a null pointer causes an illegal instruction

Proposed Fix

Add a null check before accessing the root element:

const XMLElement* xml_root = doc->RootElement();
if (!xml_root)
{
    throw RuntimeError("Invalid XML: missing root element");
}
auto format = xml_root->Attribute("BTCPP_format");

Impact

• Could lead to crashes when processing malformed XML input
• Potential security issue if the XML parser is exposed to untrusted input
• Affects error handling robustness

Additional Notes

This was found via fuzzing with AFL++ and AddressSanitizer. The crash is reproducible in both Debug and Release builds.

[facontidavide]

thanks a lot. fixed