Fix to checking for encoded CBOR larger than UINT32_MAX. Make tests work on 32-bit machine
diff --git a/inc/qcbor.h b/inc/qcbor.h
index 897346c..3011cc6 100644
--- a/inc/qcbor.h
+++ b/inc/qcbor.h
@@ -1,6 +1,6 @@
/*==============================================================================
Copyright (c) 2016-2018, The Linux Foundation.
- Copyright (c) 2018, Laurence Lundblade.
+ Copyright (c) 2018-2019, Laurence Lundblade.
All rights reserved.
Redistribution and use in source and binary forms, with or without
@@ -90,6 +90,14 @@
#define QCBOR_MAX_ARRAY_NESTING1 15 // Do not increase this over 255
+/* The largest offset to the start of an array or map. It is slightly
+ less than UINT32_MAX so the error condition can be tests on 32-bit machines.
+ UINT32_MAX comes from uStart in QCBORTrackNesting being a uin32_t.
+
+ This will cause trouble on a machine where size_t is less than 32-bits.
+ */
+#define QCBOR_MAX_ARRAY_OFFSET (UINT32_MAX - 100)
+
/*
PRIVATE DATA STRUCTURE
@@ -504,8 +512,8 @@
- Epoch dates limited to INT64_MAX (+/- 292 billion years)
- Tags on labels are ignored during decoding
- This implementation is intended to run on 32 and 64-bit CPUs. It
- will probably work on 16-bit CPUs but less efficiently.
+ This implementation is intended to run on 32 and 64-bit CPUs. Minor
+ modifications are needed for it to work on 16-bit CPUs.
The public interface uses size_t for all lengths. Internally the
implementation uses 32-bit lengths by design to use less memory and
@@ -575,8 +583,7 @@
result in this error. */
QCBOR_ERR_HIT_END,
- /** During encoding, the length of the input buffer was too large. This might
- happen on a 64-bit machine when a buffer larger than UINT32_MAX is passed.
+ /** During encoding, the length of the encoded CBOR exceeded UINT32_MAX.
*/
QCBOR_ERR_BUFFER_TOO_LARGE,
@@ -639,7 +646,7 @@
never happen on a machine with 64-bit or smaller pointers. Fixing
it is probably by increasing QCBOR_DECODE_MIN_MEM_POOL_SIZE. */
QCBOR_ERR_MEM_POOL_INTERNAL
-
+
} QCBORError;
diff --git a/src/qcbor_encode.c b/src/qcbor_encode.c
index 5bab661..460dd85 100644
--- a/src/qcbor_encode.c
+++ b/src/qcbor_encode.c
@@ -1,6 +1,6 @@
/*==============================================================================
Copyright (c) 2016-2018, The Linux Foundation.
- Copyright (c) 2018, Laurence Lundblade.
+ Copyright (c) 2018-2019, Laurence Lundblade.
All rights reserved.
Redistribution and use in source and binary forms, with or without
@@ -475,11 +475,24 @@
// Add one item to the nesting level we are in for the new map or array
me->uError = Nesting_Increment(&(me->nesting));
if(me->uError == QCBOR_SUCCESS) {
+ // The offset where the length of an array or map will get written
+ // is stored in a uint32_t, not a size_t to keep stack usage smaller. This
+ // checks to be sure there is no wrap around when recording the offset.
+ // Note that on 64-bit machines CBOR larger than 4GB can be encoded as long as no
+ // array / map offsets occur past the 4GB mark, but the public interface
+ // says that the maximum is 4GB to keep the discussion simpler.
size_t uEndPosition = UsefulOutBuf_GetEndPosition(&(me->OutBuf));
- if(uEndPosition >= UINT32_MAX-sizeof(uint64_t)) {
+
+ // QCBOR_MAX_ARRAY_OFFSET is slightly less than UINT32_MAX so this
+ // code can run on a 32-bit machine and tests can pass on a 32-bit
+ // machine. If it was exactly UINT32_MAX, then this code would
+ // not compile or run on a 32-bit machine and an #ifdef or some
+ // machine size detection would be needed reducing portability.
+ if(uEndPosition >= QCBOR_MAX_ARRAY_OFFSET) {
me->uError = QCBOR_ERR_BUFFER_TOO_LARGE;
+
} else {
- // Increase nesting level because this is a map or array
+ // Increase nesting level because this is a map or array.
// Cast from size_t to uin32_t is safe because of check above
me->uError = Nesting_Increase(&(me->nesting), uMajorType, (uint32_t)uEndPosition);
}
diff --git a/test/qcbor_encode_tests.c b/test/qcbor_encode_tests.c
index bd7a7ae..6153b24 100644
--- a/test/qcbor_encode_tests.c
+++ b/test/qcbor_encode_tests.c
@@ -1873,31 +1873,44 @@
// ------ Test for QCBOR_ERR_BUFFER_TOO_LARGE ------
- // Do all of these tests with NULL buffers as buffers would have to 4GB
- UsefulBuf Giant = (UsefulBuf){NULL, (uint64_t)UINT32_MAX + ((uint64_t)UINT32_MAX / 2)};
+ // Do all of these tests with NULL buffers so no actual large allocations are neccesary
+ UsefulBuf Buffer = (UsefulBuf){NULL, UINT32_MAX};
- // First verify no error from a really big buffer
- QCBOREncode_Init(&EC, Giant);
+ // First verify no error from a big buffer
+ QCBOREncode_Init(&EC, Buffer);
QCBOREncode_OpenArray(&EC);
- QCBOREncode_AddBytes(&EC, (UsefulBufC){NULL, (uint64_t)UINT32_MAX + 10000ULL});
+ // 6 is the CBOR overhead for opening the array and encodng the length
+ // This exactly fills the buffer.
+ QCBOREncode_AddBytes(&EC, (UsefulBufC){NULL, UINT32_MAX-6});
QCBOREncode_CloseArray(&EC);
size_t xx;
if(QCBOREncode_FinishGetSize(&EC, &xx) != QCBOR_SUCCESS) {
return -1;
}
- // second verify error from an array in encoded output too large
- QCBOREncode_Init(&EC, Giant);
+ // Second verify error from an array in encoded output too large
+ QCBOREncode_Init(&EC, Buffer);
QCBOREncode_OpenArray(&EC);
- QCBOREncode_AddBytes(&EC, (UsefulBufC){NULL, (uint64_t)UINT32_MAX+10000ULL});
- QCBOREncode_OpenArray(&EC);
+ QCBOREncode_AddBytes(&EC, (UsefulBufC){NULL, UINT32_MAX-6});
+ QCBOREncode_OpenArray(&EC); // Where QCBOR internally encounters and records error
QCBOREncode_CloseArray(&EC);
QCBOREncode_CloseArray(&EC);
if(QCBOREncode_FinishGetSize(&EC, &xx) != QCBOR_ERR_BUFFER_TOO_LARGE) {
return -2;
}
-
+ // Third, fit an array in exactly at max position allowed
+ QCBOREncode_Init(&EC, Buffer);
+ QCBOREncode_OpenArray(&EC);
+ QCBOREncode_AddBytes(&EC, (UsefulBufC){NULL, QCBOR_MAX_ARRAY_OFFSET-6});
+ QCBOREncode_OpenArray(&EC);
+ QCBOREncode_CloseArray(&EC);
+ QCBOREncode_CloseArray(&EC);
+ if(QCBOREncode_FinishGetSize(&EC, &xx) != QCBOR_SUCCESS) {
+ return -10;
+ }
+
+
// ----- QCBOR_ERR_BUFFER_TOO_SMALL --------------
// Work close to the 4GB size limit for a better test
const uint32_t uLargeSize = UINT32_MAX - 1024;
@@ -1991,3 +2004,4 @@
return 0;
}
+