fix: Prevent class construction from freed data (stack.h)

[junwha]

Hi, I found a use-after-free bug in the unit test whose root cause can be critical.

Description

The problem is that PushUnsafe in include/rapidjson/internal/stack.h doesn't flush the remained data before it pushes and allocate the new region to the class T.

template<typename T>
  RAPIDJSON_FORCEINLINE T* PushUnsafe(size_t count = 1) {
      RAPIDJSON_ASSERT(stackTop_);
      RAPIDJSON_ASSERT(static_cast<std::ptrdiff_t>(sizeof(T) * count) <= (stackEnd_ - stackTop_));
      T* ret = reinterpret_cast<T*>(stackTop_);
      stackTop_ += sizeof(T) * count;
      return ret;
  }

As you named it PushUnsafe, it could be okay to remain this function as unsafe, but RAPIDJSON_FORCEINLINE T* Push(size_t count = 1) also calls this function without flushing the data. Thus, any class using this class Stack with Push and Pop will suffer from use-after-free.

Patch

We can patch this by flushing the data with memset before Push.

Detailed Information

• OS: Ubuntu 22.04
• build command: mkdir build && cd build && cmake ../ && make -j$(nproc)
• PoC: ./bin/unittest --gtest_filter=SchemaValidator.TestSuite

At schematest.cpp:2256, the SchemaDocumentType is defined, which ends its lifetime at the end of the current for loop (schematest.cpp:2278).

for (Value::ConstValueIterator schemaItr = d.Begin(); schemaItr != d.End(); ++schemaItr) {
                    {
                        const char* description1 = (*schemaItr)["description"].GetString();
                        //printf("\ncompiling schema for json test %s \n", description1);
                        SchemaDocumentType schema((*schemaItr)["schema"], filenames[i], static_cast<SizeType>(strlen(filenames[i])), &provider, &schemaAllocator); <- allocated
                        GenericSchemaValidator<SchemaDocumentType, BaseReaderHandler<UTF8<> >, MemoryPoolAllocator<> > validator(schema, &validatorAllocator);
                        const Value& tests = (*schemaItr)["tests"];
                        for (Value::ConstValueIterator testItr = tests.Begin(); testItr != tests.End(); ++testItr) {
                            const char* description2 = (*testItr)["description"].GetString();
                            //printf("running json test %s \n", description2);
                            if (!onlyRunDescription || strcmp(description2, onlyRunDescription) == 0) {
                                const Value& data = (*testItr)["data"];
                                bool expected = (*testItr)["valid"].GetBool();
                                testCount++;
                                validator.Reset();
                                data.Accept(validator);
                                bool actual = validator.IsValid();
                                if (expected != actual)
                                    printf("Fail: %30s \"%s\" \"%s\"\n", filename, description1, description2);
                                else {
                                    //printf("Passed: %30s \"%s\" \"%s\"\n", filename, description1, description2);
                                    passCount++;
                                }
                            }
                        }
                        //printf("%zu %zu %zu\n", documentAllocator.Size(), schemaAllocator.Size(), validatorAllocator.Size());
                    } // <- freed
                 ...

Inside the constructor of the SchemaDocumentType -> Schema, the SchemaEntry instance is created by the Push method of the class Stack. At the destructor of the SchemaDocumentType, the SchemaEntry instance is freed with the Pop method of the class Stack.

Schema(SchemaDocumentType* schemaDocument, const PointerType& p, const ValueType& value, const ValueType& document, AllocatorType* allocator, const UriType& id = UriType()) :
...
					typedef typename SchemaDocumentType::SchemaEntry SchemaEntry;
          SchemaEntry *entry = schemaDocument->schemaMap_.template Push<SchemaEntry>();
          new (entry) SchemaEntry(pointer_, this, true, allocator_);

The problem occurs here, because at the second loop, the other SchemaEntry instance is constructed on the lastly popped SchemaEntry instance without flushing. Thus, there occurs use-after-free during the construction of the SchemaEntry from the second loop of the for.

Thank you for investing your valuable time in reviewing this Pull Request! :)