Null pointer dereference in ImportBlackboardFromJSON when entry creation fails

[cktii]

Description

A null pointer dereference occurs in ImportBlackboardFromJSON when attempting to use an entry pointer without verifying that the entry was successfully created. This happens because the code assumes createEntry followed by getEntry will always return a valid entry.

BehaviorTree.CPP/src/blackboard.cpp

Lines 280 to 295 in 48f6c5b

	void ImportBlackboardFromJSON(const nlohmann::json& json, Blackboard& blackboard)
	{
	for(auto it = json.begin(); it != json.end(); ++it)
	{
	if(auto res = JsonExporter::get().fromJson(it.value()))
	{
	auto entry = blackboard.getEntry(it.key());
	if(!entry)
	{
	blackboard.createEntry(it.key(), res->second);
	entry = blackboard.getEntry(it.key());
	}
	entry->value = res->first;
	}
	}
	}

Found in commit: 48f6c5b

Bug Class

Memory Safety - Null Pointer Dereference

Root Cause

void ImportBlackboardFromJSON(const nlohmann::json& json, Blackboard& blackboard)
{
  for(auto it = json.begin(); it != json.end(); ++it)
  {
    if(auto res = JsonExporter::get().fromJson(it.value()))
    {
      auto entry = blackboard.getEntry(it.key());
      if(!entry)
      {
        blackboard.createEntry(it.key(), res->second);
        entry = blackboard.getEntry(it.key());  // Could still return nullptr
      }
      entry->value = res->first;  // CRASH: Null pointer dereference
    }
  }
}

The issue occurs because:

• Code checks if initial getEntry returns nullptr
• Attempts to create entry if it doesn't exist
• Gets entry again but doesn't verify it's valid
• Dereferences the entry pointer without checking

GDB

pwndbg> p entry->value
Cannot access memory at address 0x0
pwndbg> p entry
$1 = std::shared_ptr<BT::Blackboard::Entry> (empty) = {
  get() = 0x0
}
pwndbg> p res->first
$2 = {
  _any = {
    storage = {
      dynamic = 0xffffffffe010006e,
      stack = {
        __data = "n\000\020\340\377\377\377\377\000\000\000\000\000\000\000",
        __align = {<No data fields>}
      }
    },
    vtable = 0x555556412f40 <linb::any::vtable_for_type<long>()::table>
  },
  _original_type = {
    _M_target = 0x7ffff7c6e610 <typeinfo for long>
  }
}
pwndbg> p json
$1 = (const nlohmann::json_abi_v3_11_3::json &) @0x7ffff550dc20: {
  <nlohmann::json_abi_v3_11_3::detail::json_default_base> = {<No data fields>},
  members of nlohmann::json_abi_v3_11_3::basic_json<std::map, std::vector, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, bool, long, unsigned long, double, std::allocator, nlohmann::json_abi_v3_11_3::adl_serializer, std::vector<unsigned char, std::allocator<unsigned char> >, void>:
  m_data = {
    m_type = nlohmann::json_abi_v3_11_3::detail::value_t::object,
    m_value = {
      object = 0x504000000250,
      array = 0x504000000250,
      string = 0x504000000250,
      binary = 0x504000000250,
      boolean = 80,
      number_integer = 88235808129616,
      number_unsigned = 88235808129616,
      number_float = 4.3594281529883041e-310
    }
  }
}

Stack trace

#0  0x00005555558f5d1e in BT::ImportBlackboardFromJSON (json=..., blackboard=...)

Proposed Fix

void ImportBlackboardFromJSON(const nlohmann::json& json, Blackboard& blackboard)
{
  for(auto it = json.begin(); it != json.end(); ++it)
  {
    if(auto res = JsonExporter::get().fromJson(it.value()))
    {
      auto entry = blackboard.getEntry(it.key());
      if(!entry)
      {
        blackboard.createEntry(it.key(), res->second);
        entry = blackboard.getEntry(it.key());
        if(!entry) {
          // Either throw or continue to next entry
          continue;  
        }
      }
      entry->value = res->first;
    }
  }
}

Impact

Impact

• Program crash on malformed input
• Potential security vulnerability if JSON input is untrusted
• Could be used for denial of service attacks

[facontidavide]

Thanks for the accurate description.

The more interesting question here is why "blackboard.createEntry(it.key(), res->second);" is not doing what is expected to do.

Do you have perhaps a simple example to reproduce the issue?

[cktii]

I must have a reproducer lying around that I used when debugging this. I’ll try to attach one ASAP.

While I’m at it, I’ll try to deliver a minimal PoC for all the other ones as well!

[cktii]

Okay, after some debugging I came to the conclusion that unlike stated above in the original description suggested this was a simple null pointer dereference from createEntry/getEntry, this seems to be a more complex issue involving the any type system.

Minimal Reproducer

#include "behaviortree_cpp/blackboard.h"
#include <iostream>
#include <nlohmann/json.hpp>

int main() {
    try {
        auto bb = BT::Blackboard::create();
        
        // Set initial value 
        bb->set("mmmmmm5CnCaaG", -535822226);
        
        // Create JSON that will trigger the crash
        std::string json_str = R"({
            "@@@@@@@@@@@@@@@": -535822226,
            "mmmmmm5CnCaaG": null
        })";
        
        // Parse and import JSON - will crash
        auto json = nlohmann::json::parse(json_str);
        BT::ImportBlackboardFromJSON(json, *bb);
    }
    catch(const std::exception& e) {
        std::cerr << "Exception: " << e.what() << std::endl;
        return 1;
    }
    return 0;
}

Updated stack-trace

AddressSanitizer:DEADLYSIGNAL
=================================================================
==3669987==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000010
    #0 0x7dfae074647f in linb::any::swap(linb::any&) /home/user/BehaviorTree.CPP/include/behaviortree_cpp/contrib/any.hpp:183:32
    #1 0x7dfae074647f in linb::any::operator=(linb::any const&) /home/user/BehaviorTree.CPP/include/behaviortree_cpp/contrib/any.hpp:129:18
    #2 0x7dfae074647f in BT::Any::operator=(BT::Any const&) /home/user/BehaviorTree.CPP/include/behaviortree_cpp/utils/safe_any.hpp:306:14
    #3 0x7dfae074647f in BT::ImportBlackboardFromJSON(...) /home/user/BehaviorTree.CPP/src/blackboard.cpp:292:20

The issue appears to be related to type handling in the Any implementation when dealing with null values during JSON deserialization. This suggests the fix might need to handle:

• Proper null value handling in the JSON import
• Type safety checks in the Any implementation
• Validation of entry state before assignment

[Aglargil]

#include "behaviortree_cpp/blackboard.h"
#include <iostream>
#include <nlohmann/json.hpp>

int main() {
    try {
        auto bb = BT::Blackboard::create();
        
        // Set initial value 
        bb->set("mmmmmm5CnCaaG", -535822226);
        
        // Create JSON that will trigger the crash
        std::string json_str = R"({
            "@@@@@@@@@@@@@@@": -535822226,
            "mmmmmm5CnCaaG": null
        })";
        
        // Parse and import JSON - will crash
        auto json = nlohmann::json::parse(json_str);
        BT::ImportBlackboardFromJSON(json, *bb);
    }
    catch(const std::exception& e) {
        std::cerr << "Exception: " << e.what() << std::endl;
        return 1;
    }
    return 0;
}

Hi, @facontidavide

The reason why blackboard.createEntry(it.key(), res->second); did not perform as expected in this case is: When the current key was "@@@@@@@@@@@@@@@" (15 @ symbols), since the key starts with @, rootBlackboard()->createEntryImpl(key.substr(1, key.size() - 1), info); created an entry with key "@@@@@@@@@@@@@@" (14 @ symbols). The critical issue was that the implementation didn't recursively call createEntry during this process, while the getEntry method does employ recursive calls to getEntry. This inconsistency ultimately led to failing to find the entry when searching for an empty string key (after recursively stripping all @ prefixes).

I've submitted a PR to fix this behavior #929

[facontidavide]

fixed making that syntax illegal, as it should be

[facontidavide]

thanks