Null out mjData.buffer and mjData.stack before possibly calling mj_deleteData.

While creating an mjData, any error triggers mj_deleteData. If buffer or stack is not NULL at that point, an uninitialized pointer will be freed.

PiperOrigin-RevId: 465378329
Change-Id: I9be0eef0648e05e3e5f1361da346a5046485cde7
This commit is contained in:
Nimrod Gileadi
2022-08-04 12:36:07 -07:00
committed by Copybara-Service
parent 0e8fd182de
commit 4268d81b55
2 changed files with 28 additions and 1 deletions
+4 -1
View File
@@ -838,6 +838,7 @@ static mjData* _makeData(const mjModel* m) {
// compute buffer size
d->nbuffer = 0;
d->buffer = d->stack = NULL;
#define X(type, name, nr, nc) \
if (!safeAddToBufferSize(&offset, &d->nbuffer, sizeof(type), m->nr, nc)) { \
mju_warning("Invalid data: " #name " too large."); \
@@ -871,7 +872,9 @@ static mjData* _makeData(const mjModel* m) {
mjData* mj_makeData(const mjModel* m) {
mjData* d = _makeData(m);
mj_resetData(m, d);
if (d) {
mj_resetData(m, d);
}
return d;
}
+24
View File
@@ -17,6 +17,7 @@
#include "src/engine/engine_io.h"
#include <array>
#include <climits>
#include <cstring>
#include <string>
@@ -24,6 +25,7 @@
#include <gtest/gtest.h>
#include <absl/strings/str_format.h>
#include <mujoco/mjxmacro.h>
#include "src/engine/engine_util_errmem.h"
#include "test/fixture.h"
namespace mujoco {
@@ -177,6 +179,28 @@ TEST_F(EngineIoTest, CopyDataWithPartialModel) {
mj_deleteData(copy);
}
TEST_F(EngineIoTest, MakeDataReturnsNullOnFailure) {
constexpr char xml[] = "<mujoco/>";
std::array<char, 1024> error;
mjModel* model = LoadModelFromString(xml, error.data(), error.size());
ASSERT_THAT(model, NotNull()) << "Failed to load model: " << error.data();
// trigger overflow intentionally
model->nbody = INT_MAX;
static bool warning = false;
warning = false;
mju_user_warning = [](const char* error) {
warning = true;
};
mjData* data = mj_makeData(model);
EXPECT_THAT(data, IsNull());
EXPECT_TRUE(warning) << "Expecting warning to be triggered.";
mj_deleteData(data);
mj_deleteModel(model);
}
using ValidateReferencesTest = MujocoTest;
TEST_F(ValidateReferencesTest, BodyReferences) {