diff --git a/include/rapidjson/document.h b/include/rapidjson/document.h index 0910122..61d031b 100644 --- a/include/rapidjson/document.h +++ b/include/rapidjson/document.h @@ -916,8 +916,13 @@ public: */ GenericValue& operator=(GenericValue& rhs) RAPIDJSON_NOEXCEPT { if (RAPIDJSON_LIKELY(this != &rhs)) { + // Can't destroy "this" before assigning "rhs", otherwise "rhs" + // could be used after free if it's an sub-Value of "this", + // hence the temporary danse. + GenericValue temp; + temp.RawAssign(rhs); this->~GenericValue(); - RawAssign(rhs); + RawAssign(temp); } return *this; } diff --git a/test/unittest/documenttest.cpp b/test/unittest/documenttest.cpp index 472165f..c3d1e48 100644 --- a/test/unittest/documenttest.cpp +++ b/test/unittest/documenttest.cpp @@ -325,6 +325,8 @@ TEST(Document, Swap) { EXPECT_TRUE(d1.IsNull()); // reset document, including allocator + // so clear o before so that it doesnt contain dangling elements + o.Clear(); Document().Swap(d2); EXPECT_TRUE(d2.IsNull()); EXPECT_NE(&d2.GetAllocator(), &a); diff --git a/test/unittest/valuetest.cpp b/test/unittest/valuetest.cpp index 00f0652..0a6b325 100644 --- a/test/unittest/valuetest.cpp +++ b/test/unittest/valuetest.cpp @@ -1078,9 +1078,9 @@ static void TestArray(T& x, Allocator& allocator) { } TEST(Value, Array) { + Value::AllocatorType allocator; Value x(kArrayType); const Value& y = x; - Value::AllocatorType allocator; EXPECT_EQ(kArrayType, x.GetType()); EXPECT_TRUE(x.IsArray()); @@ -1119,6 +1119,16 @@ TEST(Value, Array) { z.SetArray(); EXPECT_TRUE(z.IsArray()); EXPECT_TRUE(z.Empty()); + + // PR #1503: assign from inner Value + { + CrtAllocator a; // Free() is not a noop + GenericValue, CrtAllocator> nullValue; + GenericValue, CrtAllocator> arrayValue(kArrayType); + arrayValue.PushBack(nullValue, a); + arrayValue = arrayValue[0]; // shouldn't crash (use after free) + EXPECT_TRUE(arrayValue.IsNull()); + } } TEST(Value, ArrayHelper) { @@ -1481,9 +1491,9 @@ static void TestObject(T& x, Allocator& allocator) { } TEST(Value, Object) { + Value::AllocatorType allocator; Value x(kObjectType); const Value& y = x; // const version - Value::AllocatorType allocator; EXPECT_EQ(kObjectType, x.GetType()); EXPECT_TRUE(x.IsObject());