Tencent / rapidjson

A fast JSON parser/generator for C++ with both SAX/DOM style API
http://rapidjson.org/
Other
14k stars 3.5k forks source link

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

Open junwha0511 opened 5 months ago

junwha0511 commented 5 months ago

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

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! :)