Skip to content

Commit 2deed25

Browse files
author
David LeBlanc
committed
Fix for issue 775
Fix possibly incorrect integer overflow check with an efficient correct check. Not fixing the issue where size == 0, unsure if this is by design, or what error to return if not.
1 parent 419877c commit 2deed25

File tree

1 file changed

+20
-4
lines changed

1 file changed

+20
-4
lines changed

src/unpack.c

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -190,19 +190,27 @@ static inline int template_callback_false(unpack_user* u, msgpack_object* o)
190190
static inline int template_callback_array(unpack_user* u, unsigned int n, msgpack_object* o)
191191
{
192192
unsigned int size;
193+
unsigned long long tmp;
194+
193195
o->type = MSGPACK_OBJECT_ARRAY;
194196
o->via.array.size = 0;
195-
size = n*sizeof(msgpack_object);
196-
if (size / sizeof(msgpack_object) != n) {
197+
tmp = (unsigned long long)n * sizeof(msgpack_object);
198+
199+
if (tmp & 0xffffffff00000000) {
197200
// integer overflow
198201
return MSGPACK_UNPACK_NOMEM_ERROR;
199202
}
203+
204+
size = (unsigned int)tmp;
205+
200206
if (*u->z == NULL) {
201207
*u->z = msgpack_zone_new(MSGPACK_ZONE_CHUNK_SIZE);
202208
if(*u->z == NULL) {
203209
return MSGPACK_UNPACK_NOMEM_ERROR;
204210
}
205211
}
212+
213+
// Unsure whether size = 0 should be an error, and if so, what to return
206214
o->via.array.ptr = (msgpack_object*)msgpack_zone_malloc(*u->z, size);
207215
if(o->via.array.ptr == NULL) { return MSGPACK_UNPACK_NOMEM_ERROR; }
208216
return 0;
@@ -223,19 +231,27 @@ static inline int template_callback_array_item(unpack_user* u, msgpack_object* c
223231
static inline int template_callback_map(unpack_user* u, unsigned int n, msgpack_object* o)
224232
{
225233
unsigned int size;
234+
unsigned long long tmp;
235+
226236
o->type = MSGPACK_OBJECT_MAP;
227237
o->via.map.size = 0;
228-
size = n*sizeof(msgpack_object_kv);
229-
if (size / sizeof(msgpack_object_kv) != n) {
238+
size = (unsigned long long)n * sizeof(msgpack_object_kv);
239+
240+
if (tmp & 0xffffffff00000000) {
230241
// integer overflow
231242
return MSGPACK_UNPACK_NOMEM_ERROR;
232243
}
244+
245+
size = (unsigned int)tmp;
246+
233247
if (*u->z == NULL) {
234248
*u->z = msgpack_zone_new(MSGPACK_ZONE_CHUNK_SIZE);
235249
if(*u->z == NULL) {
236250
return MSGPACK_UNPACK_NOMEM_ERROR;
237251
}
238252
}
253+
254+
// Should size = 0 be an error? If so, what error to return?
239255
o->via.map.ptr = (msgpack_object_kv*)msgpack_zone_malloc(*u->z, size);
240256
if(o->via.map.ptr == NULL) { return MSGPACK_UNPACK_NOMEM_ERROR; }
241257
return 0;

0 commit comments

Comments
 (0)