From feaf03421dda0213382b51aff07bd5a96b29487b Mon Sep 17 00:00:00 2001
From: Fabio Alessandrelli <fabio.alessandrelli@gmail.com>
Date: Sun, 8 Jul 2018 15:11:41 +0200
Subject: Fix marshalls size checks.
Yesterday, when playing around with my network code, I realized there is
a security issue in decode_variant, at least when decoding PoolArrays.
Basically, the size of the PoolArray is encoded in a uint32_t, when
decoding it, that value is cast to int when comparing if the packet is
actually that size causing numbers with MSB=1 to be interpreted as
negative thus always passing the check. That same value though, is used
as uint32_t again to resize the output vector. For this reason, sending
a malformed packet with declared type PoolByteArray and size of 2^31(+x)
causes the engine to try to allocate 2+GB of pool memory, causing the
engine to crash.
(cherry picked from commit 5262d1bbcc81a06db66ac45c3f75535f231268bc)
---
core/io/marshalls.cpp | 177 ++++++++++++++++++++++----------------------------
1 file changed, 76 insertions(+), 101 deletions(-)
diff --git a/core/io/marshalls.cpp b/core/io/marshalls.cpp
index 0a3a6c1ba1..e97df0c261 100644
--- a/core/io/marshalls.cpp
+++ b/core/io/marshalls.cpp
@@ -32,8 +32,13 @@
#include "os/keyboard.h"
#include "print_string.h"
#include "reference.h"
+#include <limits.h>
#include <stdio.h>
+#define _S(a) ((int32_t)a)
+#define ERR_FAIL_ADD_OF(a, b, err) ERR_FAIL_COND_V(_S(b) < 0 || _S(a) < 0 || _S(a) > INT_MAX - _S(b), err)
+#define ERR_FAIL_MUL_OF(a, b, err) ERR_FAIL_COND_V(_S(a) < 0 || _S(b) <= 0 || _S(a) > INT_MAX / _S(b), err)
+
void EncodedObjectAsID::_bind_methods() {
ClassDB::bind_method(D_METHOD("set_object_id", "id"), &EncodedObjectAsID::set_object_id);
ClassDB::bind_method(D_METHOD("get_object_id"), &EncodedObjectAsID::get_object_id);
@@ -60,23 +65,31 @@ EncodedObjectAsID::EncodedObjectAsID() {
static Error _decode_string(const uint8_t *&buf, int &len, int *r_len, String &r_string) {
ERR_FAIL_COND_V(len < 4, ERR_INVALID_DATA);
- uint32_t strlen = decode_uint32(buf);
+ int32_t strlen = decode_uint32(buf);
+ int32_t pad = 0;
+
+ // Handle padding
+ if (strlen % 4) {
+ pad = 4 - strlen % 4;
+ }
+
buf += 4;
len -= 4;
- ERR_FAIL_COND_V((int)strlen > len, ERR_FILE_EOF);
+
+ // Ensure buffer is big enough
+ ERR_FAIL_ADD_OF(strlen, pad, ERR_FILE_EOF);
+ ERR_FAIL_COND_V(strlen < 0 || strlen + pad > len, ERR_FILE_EOF);
String str;
- str.parse_utf8((const char *)buf, strlen);
+ ERR_FAIL_COND_V(str.parse_utf8((const char *)buf, strlen), ERR_INVALID_DATA);
r_string = str;
- //handle padding
- if (strlen % 4) {
- strlen += 4 - strlen % 4;
- }
+ // Add padding
+ strlen += pad;
+ // Update buffer pos, left data count, and return size
buf += strlen;
len -= strlen;
-
if (r_len) {
(*r_len) += 4 + strlen;
}
@@ -119,14 +132,15 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
} break;
case Variant::INT: {
- ERR_FAIL_COND_V(len < 4, ERR_INVALID_DATA);
if (type & ENCODE_FLAG_64) {
+ ERR_FAIL_COND_V(len < 8, ERR_INVALID_DATA);
int64_t val = decode_uint64(buf);
r_variant = val;
if (r_len)
(*r_len) += 8;
} else {
+ ERR_FAIL_COND_V(len < 4, ERR_INVALID_DATA);
int32_t val = decode_uint32(buf);
r_variant = val;
if (r_len)
@@ -136,14 +150,14 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
} break;
case Variant::REAL: {
- ERR_FAIL_COND_V(len < (int)4, ERR_INVALID_DATA);
-
if (type & ENCODE_FLAG_64) {
+ ERR_FAIL_COND_V(len < 8, ERR_INVALID_DATA);
double val = decode_double(buf);
r_variant = val;
if (r_len)
(*r_len) += 8;
} else {
+ ERR_FAIL_COND_V(len < 4, ERR_INVALID_DATA);
float val = decode_float(buf);
r_variant = val;
if (r_len)
@@ -164,7 +178,7 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
// math types
case Variant::VECTOR2: {
- ERR_FAIL_COND_V(len < (int)4 * 2, ERR_INVALID_DATA);
+ ERR_FAIL_COND_V(len < 4 * 2, ERR_INVALID_DATA);
Vector2 val;
val.x = decode_float(&buf[0]);
val.y = decode_float(&buf[4]);
@@ -176,7 +190,7 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
} break; // 5
case Variant::RECT2: {
- ERR_FAIL_COND_V(len < (int)4 * 4, ERR_INVALID_DATA);
+ ERR_FAIL_COND_V(len < 4 * 4, ERR_INVALID_DATA);
Rect2 val;
val.position.x = decode_float(&buf[0]);
val.position.y = decode_float(&buf[4]);
@@ -190,7 +204,7 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
} break;
case Variant::VECTOR3: {
- ERR_FAIL_COND_V(len < (int)4 * 3, ERR_INVALID_DATA);
+ ERR_FAIL_COND_V(len < 4 * 3, ERR_INVALID_DATA);
Vector3 val;
val.x = decode_float(&buf[0]);
val.y = decode_float(&buf[4]);
@@ -203,7 +217,7 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
} break;
case Variant::TRANSFORM2D: {
- ERR_FAIL_COND_V(len < (int)4 * 6, ERR_INVALID_DATA);
+ ERR_FAIL_COND_V(len < 4 * 6, ERR_INVALID_DATA);
Transform2D val;
for (int i = 0; i < 3; i++) {
for (int j = 0; j < 2; j++) {
@@ -220,7 +234,7 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
} break;
case Variant::PLANE: {
- ERR_FAIL_COND_V(len < (int)4 * 4, ERR_INVALID_DATA);
+ ERR_FAIL_COND_V(len < 4 * 4, ERR_INVALID_DATA);
Plane val;
val.normal.x = decode_float(&buf[0]);
val.normal.y = decode_float(&buf[4]);
@@ -234,7 +248,7 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
} break;
case Variant::QUAT: {
- ERR_FAIL_COND_V(len < (int)4 * 4, ERR_INVALID_DATA);
+ ERR_FAIL_COND_V(len < 4 * 4, ERR_INVALID_DATA);
Quat val;
val.x = decode_float(&buf[0]);
val.y = decode_float(&buf[4]);
@@ -248,7 +262,7 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
} break;
case Variant::AABB: {
- ERR_FAIL_COND_V(len < (int)4 * 6, ERR_INVALID_DATA);
+ ERR_FAIL_COND_V(len < 4 * 6, ERR_INVALID_DATA);
AABB val;
val.position.x = decode_float(&buf[0]);
val.position.y = decode_float(&buf[4]);
@@ -264,7 +278,7 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
} break;
case Variant::BASIS: {
- ERR_FAIL_COND_V(len < (int)4 * 9, ERR_INVALID_DATA);
+ ERR_FAIL_COND_V(len < 4 * 9, ERR_INVALID_DATA);
Basis val;
for (int i = 0; i < 3; i++) {
for (int j = 0; j < 3; j++) {
@@ -281,7 +295,7 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
} break;
case Variant::TRANSFORM: {
- ERR_FAIL_COND_V(len < (int)4 * 12, ERR_INVALID_DATA);
+ ERR_FAIL_COND_V(len < 4 * 12, ERR_INVALID_DATA);
Transform val;
for (int i = 0; i < 3; i++) {
for (int j = 0; j < 3; j++) {
@@ -303,7 +317,7 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
// misc types
case Variant::COLOR: {
- ERR_FAIL_COND_V(len < (int)4 * 4, ERR_INVALID_DATA);
+ ERR_FAIL_COND_V(len < 4 * 4, ERR_INVALID_DATA);
Color val;
val.r = decode_float(&buf[0]);
val.g = decode_float(&buf[4]);
@@ -318,7 +332,7 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
case Variant::NODE_PATH: {
ERR_FAIL_COND_V(len < 4, ERR_INVALID_DATA);
- uint32_t strlen = decode_uint32(buf);
+ int32_t strlen = decode_uint32(buf);
if (strlen & 0x80000000) {
//new format
@@ -343,31 +357,15 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
for (uint32_t i = 0; i < total; i++) {
- ERR_FAIL_COND_V((int)len < 4, ERR_INVALID_DATA);
- strlen = decode_uint32(buf);
-
- int pad = 0;
-
- if (strlen % 4)
- pad += 4 - strlen % 4;
-
- buf += 4;
- len -= 4;
- ERR_FAIL_COND_V((int)strlen + pad > len, ERR_INVALID_DATA);
-
String str;
- str.parse_utf8((const char *)buf, strlen);
+ Error err = _decode_string(buf, len, r_len, str);
+ if (err)
+ return err;
if (i < namecount)
names.push_back(str);
else
subnames.push_back(str);
-
- buf += strlen + pad;
- len -= strlen + pad;
-
- if (r_len)
- (*r_len) += 4 + strlen + pad;
}
r_variant = NodePath(names, subnames, flags & 1);
@@ -375,17 +373,7 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
} else {
//old format, just a string
- buf += 4;
- len -= 4;
- ERR_FAIL_COND_V((int)strlen > len, ERR_INVALID_DATA);
-
- String str;
- str.parse_utf8((const char *)buf, strlen);
-
- r_variant = NodePath(str);
-
- if (r_len)
- (*r_len) += 4 + strlen;
+ ERR_FAIL_V(ERR_INVALID_DATA);
}
} break;
@@ -402,6 +390,7 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
if (type & ENCODE_FLAG_OBJECT_AS_ID) {
//this _is_ allowed
+ ERR_FAIL_COND_V(len < 8, ERR_INVALID_DATA);
ObjectID val = decode_uint64(buf);
if (r_len)
(*r_len) += 8;
@@ -475,7 +464,7 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
case Variant::DICTIONARY: {
ERR_FAIL_COND_V(len < 4, ERR_INVALID_DATA);
- uint32_t count = decode_uint32(buf);
+ int32_t count = decode_uint32(buf);
// bool shared = count&0x80000000;
count &= 0x7FFFFFFF;
@@ -488,7 +477,7 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
Dictionary d;
- for (uint32_t i = 0; i < count; i++) {
+ for (int i = 0; i < count; i++) {
Variant key, value;
@@ -520,7 +509,7 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
case Variant::ARRAY: {
ERR_FAIL_COND_V(len < 4, ERR_INVALID_DATA);
- uint32_t count = decode_uint32(buf);
+ int32_t count = decode_uint32(buf);
// bool shared = count&0x80000000;
count &= 0x7FFFFFFF;
@@ -533,7 +522,7 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
Array varr;
- for (uint32_t i = 0; i < count; i++) {
+ for (int i = 0; i < count; i++) {
int used = 0;
Variant v;
@@ -555,17 +544,17 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
case Variant::POOL_BYTE_ARRAY: {
ERR_FAIL_COND_V(len < 4, ERR_INVALID_DATA);
- uint32_t count = decode_uint32(buf);
+ int32_t count = decode_uint32(buf);
buf += 4;
len -= 4;
- ERR_FAIL_COND_V((int)count > len, ERR_INVALID_DATA);
+ ERR_FAIL_COND_V(count < 0 || count > len, ERR_INVALID_DATA);
PoolVector<uint8_t> data;
if (count) {
data.resize(count);
PoolVector<uint8_t>::Write w = data.write();
- for (uint32_t i = 0; i < count; i++) {
+ for (int32_t i = 0; i < count; i++) {
w[i] = buf[i];
}
@@ -585,10 +574,11 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
case Variant::POOL_INT_ARRAY: {
ERR_FAIL_COND_V(len < 4, ERR_INVALID_DATA);
- uint32_t count = decode_uint32(buf);
+ int32_t count = decode_uint32(buf);
buf += 4;
len -= 4;
- ERR_FAIL_COND_V((int)count * 4 > len, ERR_INVALID_DATA);
+ ERR_FAIL_MUL_OF(count, 4, ERR_INVALID_DATA);
+ ERR_FAIL_COND_V(count < 0 || count * 4 > len, ERR_INVALID_DATA);
PoolVector<int> data;
@@ -596,7 +586,7 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
//const int*rbuf=(const int*)buf;
data.resize(count);
PoolVector<int>::Write w = data.write();
- for (uint32_t i = 0; i < count; i++) {
+ for (int32_t i = 0; i < count; i++) {
w[i] = decode_uint32(&buf[i * 4]);
}
@@ -612,10 +602,11 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
case Variant::POOL_REAL_ARRAY: {
ERR_FAIL_COND_V(len < 4, ERR_INVALID_DATA);
- uint32_t count = decode_uint32(buf);
+ int32_t count = decode_uint32(buf);
buf += 4;
len -= 4;
- ERR_FAIL_COND_V((int)count * 4 > len, ERR_INVALID_DATA);
+ ERR_FAIL_MUL_OF(count, 4, ERR_INVALID_DATA);
+ ERR_FAIL_COND_V(count < 0 || count * 4 > len, ERR_INVALID_DATA);
PoolVector<float> data;
@@ -623,7 +614,7 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
//const float*rbuf=(const float*)buf;
data.resize(count);
PoolVector<float>::Write w = data.write();
- for (uint32_t i = 0; i < count; i++) {
+ for (int32_t i = 0; i < count; i++) {
w[i] = decode_float(&buf[i * 4]);
}
@@ -640,7 +631,7 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
case Variant::POOL_STRING_ARRAY: {
ERR_FAIL_COND_V(len < 4, ERR_INVALID_DATA);
- uint32_t count = decode_uint32(buf);
+ int32_t count = decode_uint32(buf);
PoolVector<String> strings;
buf += 4;
@@ -650,35 +641,14 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
(*r_len) += 4;
//printf("string count: %i\n",count);
- for (int i = 0; i < (int)count; i++) {
+ for (int32_t i = 0; i < count; i++) {
- ERR_FAIL_COND_V(len < 4, ERR_INVALID_DATA);
- uint32_t strlen = decode_uint32(buf);
-
- buf += 4;
- len -= 4;
- ERR_FAIL_COND_V((int)strlen > len, ERR_INVALID_DATA);
-
- //printf("loaded string: %s\n",(const char*)buf);
String str;
- str.parse_utf8((const char *)buf, strlen);
+ Error err = _decode_string(buf, len, r_len, str);
+ if (err)
+ return err;
strings.push_back(str);
-
- buf += strlen;
- len -= strlen;
-
- if (r_len)
- (*r_len) += 4 + strlen;
-
- if (strlen % 4) {
- int pad = 4 - (strlen % 4);
- buf += pad;
- len -= pad;
- if (r_len) {
- (*r_len) += pad;
- }
- }
}
r_variant = strings;
@@ -687,11 +657,12 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
case Variant::POOL_VECTOR2_ARRAY: {
ERR_FAIL_COND_V(len < 4, ERR_INVALID_DATA);
- uint32_t count = decode_uint32(buf);
+ int32_t count = decode_uint32(buf);
buf += 4;
len -= 4;
- ERR_FAIL_COND_V((int)count * 4 * 2 > len, ERR_INVALID_DATA);
+ ERR_FAIL_MUL_OF(count, 4 * 2, ERR_INVALID_DATA);
+ ERR_FAIL_COND_V(count < 0 || count * 4 * 2 > len, ERR_INVALID_DATA);
PoolVector<Vector2> varray;
if (r_len) {
@@ -702,7 +673,7 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
varray.resize(count);
PoolVector<Vector2>::Write w = varray.write();
- for (int i = 0; i < (int)count; i++) {
+ for (int32_t i = 0; i < count; i++) {
w[i].x = decode_float(buf + i * 4 * 2 + 4 * 0);
w[i].y = decode_float(buf + i * 4 * 2 + 4 * 1);
@@ -722,11 +693,13 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
case Variant::POOL_VECTOR3_ARRAY: {
ERR_FAIL_COND_V(len < 4, ERR_INVALID_DATA);
- uint32_t count = decode_uint32(buf);
+ int32_t count = decode_uint32(buf);
buf += 4;
len -= 4;
- ERR_FAIL_COND_V((int)count * 4 * 3 > len, ERR_INVALID_DATA);
+ ERR_FAIL_MUL_OF(count, 4 * 3, ERR_INVALID_DATA);
+ ERR_FAIL_COND_V(count < 0 || count * 4 * 3 > len, ERR_INVALID_DATA);
+
PoolVector<Vector3> varray;
if (r_len) {
@@ -737,7 +710,7 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
varray.resize(count);
PoolVector<Vector3>::Write w = varray.write();
- for (int i = 0; i < (int)count; i++) {
+ for (int32_t i = 0; i < count; i++) {
w[i].x = decode_float(buf + i * 4 * 3 + 4 * 0);
w[i].y = decode_float(buf + i * 4 * 3 + 4 * 1);
@@ -758,11 +731,13 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
case Variant::POOL_COLOR_ARRAY: {
ERR_FAIL_COND_V(len < 4, ERR_INVALID_DATA);
- uint32_t count = decode_uint32(buf);
+ int32_t count = decode_uint32(buf);
buf += 4;
len -= 4;
- ERR_FAIL_COND_V((int)count * 4 * 4 > len, ERR_INVALID_DATA);
+ ERR_FAIL_MUL_OF(count, 4 * 4, ERR_INVALID_DATA);
+ ERR_FAIL_COND_V(count < 0 || count * 4 * 4 > len, ERR_INVALID_DATA);
+
PoolVector<Color> carray;
if (r_len) {
@@ -773,7 +748,7 @@ Error decode_variant(Variant &r_variant, const uint8_t *p_buffer, int p_len, int
carray.resize(count);
PoolVector<Color>::Write w = carray.write();
- for (int i = 0; i < (int)count; i++) {
+ for (int32_t i = 0; i < count; i++) {
w[i].r = decode_float(buf + i * 4 * 4 + 4 * 0);
w[i].g = decode_float(buf + i * 4 * 4 + 4 * 1);
@@ -1323,7 +1298,7 @@ Error encode_variant(const Variant &p_variant, uint8_t *r_buffer, int &r_len, bo
while (r_len % 4) {
r_len++; //pad
if (buf)
- buf++;
+ *(buf++) = 0;
}
}
--
cgit v1.2.3