Dangling reference in SchemaValidator after garbage collection, SEGFAULT

[musteresel]

lua-rapidjson/src/Schema.cpp

Lines 68 to 72 in c06b8a6

	SchemaValidator* Userdata<SchemaValidator>::construct(lua_State * L)
	{
	SchemaDocument* sd = Userdata<SchemaDocument>::check(L, 1);
	return new SchemaValidator(*sd);
	}

This creates a dangling reference if the SchemaDocument used to construct the SchemaValidator goes out of scope and is thus garbage collected. The constructor of SchemaValidator expects a reference to a SchemaDocument, and thus does not manage its lifetime nor does it create an internal copy!
The result is undefined behaviour which might represent itself as segmentation faults. Lua is not able to catch these errors.

Example code which shows this:

rj = require 'rapidjson'
collectgarbage("stop") -- We want to be in control of that for now

-- Using a function to introduce some scope here...
function CreateValidator()
    local schem = [[
{
    "type": "object",
    "properties": {
        "field": {
            "type": "string",
            "enum": [
                "frobo",
                "tobo"
                ]
        }
    }
}
]]
    local sd = rj.SchemaDocument(schem) -- this SchemaDocument is referenced by the local variable sd
    local validator = rj.SchemaValidator(sd) -- now the SchemaValidator references (in C++ not LUA) the SchemaDocument
    -- in other words, the (C++) object which is referenced by "validator" has a raw (C++) pointer to the (C++) object
    -- which is referenced by "sd"
    return validator
end -- the SchemaDocument variable sd now goes out of scope.

local vali = CreateValidator()

collectgarbage("collect") -- The SchemaDocument variable "sd" is now garbage collected
-- Since the metatable is appropriately set up, this causes the (C++) object referenced by "sd" to be deleted
-- As a result, the SchemaValidator now references a freed / no longer valid SchemaDocument
print(vali:validate(rj.Document('42'))) -- This doesn't seem to bother here
print(vali:validate(rj.Document('"answer to life"'))) -- Nor here
print(vali:validate(rj.Document('{"field": "bazoga"}'))) -- But here, since this needs to access the SchemaDocument (which has been deleted)

When run with the rock compiled with CMAKE_BUILD_TYPE=Debug this gives us:

# lua rapidjson-bug.lua 
false   invalid "type" in document at pointer "#"
false   invalid "type" in document at pointer "#"
lua: /tmp/luarocks_rapidjson-0.7.1-1-KxWzcC/lua-rapidjson/rapidjson/include/rapidjson/document.h:1857: rapidjson::SizeType rapidjson::GenericValue<Encoding, Allocator>::GetStringLength() const [with Encoding = rapidjson::UTF8<>; Allocator = rapidjson::CrtAllocator; rapidjson::SizeType = unsigned int]: Assertion `IsString()' failed.
Aborted (core dumped)

With the release build it just segfaults.

A "fix" of the above example code would be to remove the local in front of sd. This avoids sd from being garbage collected, and thus avoids the dangling reference.

[musteresel]

Even worse, with the release build from https://luarocks.org/rapidjson-0.7.1-1.src.rock the above test script gives

# lua rapidjson-bug.lua 
false   invalid "type" in document at pointer "#"
false   invalid "type" in document at pointer "#"
true

Replacing local sd = ... with sd = ... shows the correct output:

# lua rapidjson-bug.lua 
false   invalid "type" in document at pointer "#"
false   invalid "type" in document at pointer "#"
false   invalid "enum" in document at pointer "#/field"

In other words, it's undefined behaviour and you can't even rely on it crashing.