When value_to_pretty is called with recursionsLeft=0 and an Array or Map
type, container_to_pretty is called with recursionsLeft - 1 = -1. This
breaks the recursion limit check.
In practice, this can be triggered with a CBOR data containing 1023
Arrays, a Tag and many more Arrays:
$ python3 -c 'import sys;sys.stdout.buffer.write(b"\x9f" * 1023 + b"\xc0\x9f" + b"\x9f" * 100000 + b"\xff" * 101024)' | ./bin/cbordump
Segmentation fault (core dumped)
This segmentation fault is due to the stack growing too much, due to the
quantity of recursive calls.
Fix this by reporting a proper error when recursionsLeft <= 0, instead
of when recursionsLeft == 0. The same input now produces:
[_ [_ [_ ... [_ 0([
-: internal error: too many nested containers found in recursive function
_ <nesting too deep, recursion stopped>
Moreover, using fewer nested arrays works fine:
$ python3 -c 'import sys;sys.stdout.buffer.write(b"\x9f" * 1023 + b"\xc0\x9f" + b"\x9f" * 1024 + b"\xff" * 2048)' |./bin/cbordump
[_ [_ [_ ... [_ 0([_ <nesting too deep, recursion stopped>])] ... ]]]
Also modify the test when formatting CborTagType to ensure
value_to_pretty is never called with a negative recursionsLeft.
When printing CBOR data containing NaN, function convertToUint64 does an
undefined behavior. This can be reproduced using test cases from
tests/parser/data.cpp:
$ make CC='clang -fsanitize=undefined'
$ printf "\xfb\x7f\xf8\0\0\0\0\0\0" | ./bin/cbordump
src/cborpretty.c:171:17: runtime error: nan is outside the range of
representable values of type 'unsigned long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/cborpretty.c:171:17 in
nan
$ printf "\xf9\x7e\x00" | ./bin/cbordump
src/cborpretty.c:171:17: runtime error: nan is outside the range of
representable values of type 'unsigned long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/cborpretty.c:171:17 in
nan
Fix this by checking whether the value to convert is not NaN.
When function text_string_to_escaped successfully parses a string and
fails to parse the next value (cbor_value_finish_string_iteration
returns an error), it correctly propagates the error but the string is
never freed.
This can be reproduced with:
make CC='clang -g -fsanitize=address'
printf '\x82\x60\xff' | ./bin/cbordump -j
clang's Address Sanitizer reports:
=================================================================
==20317==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 1 byte(s) in 1 object(s) allocated from:
#0 0x560b654b9916 in __interceptor_realloc (/tinycbor/bin/cbordump+0xa4916) (BuildId: f9933666b5d987b21f68c2887de4aebe93bc2bef)
#1 0x560b654f5c18 in escape_text_string /tinycbor/src/cbortojson.c:331:15
#2 0x560b654f3e29 in text_string_to_escaped /tinycbor/src/cbortojson.c:377:19
#3 0x560b654f267d in value_to_json /tinycbor/src/cbortojson.c:674:19
#4 0x560b654f34c2 in array_to_json /tinycbor/src/cbortojson.c:545:25
#5 0x560b654f2085 in value_to_json /tinycbor/src/cbortojson.c:627:19
#6 0x560b654f1baf in cbor_value_to_json_advance /tinycbor/src/cbortojson.c:816:12
#7 0x560b654ea928 in dumpFile /tinycbor/tools/cbordump/cbordump.c:76:19
#8 0x560b654ead2b in main /tinycbor/tools/cbordump/cbordump.c:149:9
#9 0x7fa9d7629d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
SUMMARY: AddressSanitizer: 1 byte(s) leaked in 1 allocation(s).
Fix this by freeing the string when cbor_value_finish_string_iteration
fails.
Fixes: e072bc1d78 ("CBOR-to-JSON: do properly escape JSON strings")
Both the C and C++ standards say it is Undefined Behavior to convert a
floating point number to integer if the input is out of bounds of the
destination type.
And indeed this started failing in recent builds, with
val = 18446744073709551616 (2^64)
it has probably been producing ival = 18446744073709551615 for a while,
but the conversion back to floating point now rounded up and compared
equal to the input.
So let's just fix it.
Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
And make CBOR_PRIVATE_API fall back to it.
We also need to provide the file for the old Makefile build too.
Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
This method allows for writing raw data directly to the encoding buffer. This can be useful if you have something stored as CBOR encoded data.
Fixes#162.
Signed-off-by: Tofik Sonono <tofiksonono@msn.com>
Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
We were returning from the function with the memory we had allocated and
freed, if the second iteration over the string produced a failure that
didn't happen on the first one. This can't happen with pure memory
buffers, but can happen with an external data source that fails to
produce the same contents twice.
I'm documenting that the values in all error conditions except for OOM
are undefined, so one mustn't attempt to use them, even to free. This
does not change behaviour of the library, just documents.
But this commit does make it clear the OOM condition will return a valid
`*buflen` and `next`, the latter of which is new behaviour with this
commit.
Fixes#258.
Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
There's a discussion in the C and C++ communities whether you're allowed
to use the values of pointers that have been deallocated, if you don't
dereference them. Some argue that it is Undefined Behaviour in spite of
the numeric value stored in the variable not having changed.
Instead of arguing, let's just make sure we don't use the pointers after
they have become dangling. We only needed the offset of how far we've
written into the buffer to restore the state and we have a function that
returns exactly that.
Seen while debugging #259.
Drive-by keep the `buffersize` global variable unchanged until after
`realloc()` has returned with success.
Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
We hadn't bothered, as this was just example-like code to show how one
could convert from CBOR to JSON. But as it was added to the library (no
extra dependency), we should Do The Right Thing (DTRT) and escape.
This patch could have used cbor_value_to_pretty() to print the string,
which has better support for UTF-8 escaping and thus checks for UTF-8
correctness, but that would make map_to_json()'s metadata functionality
much more complex, especially since we cannot rely on open_memstream()
always being available. Therefore, we are partially duplicating
cborpretty.c's utf8EscapedDump().
Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
1024 levels will probably be good enough for everyone, like
cborparser.c. For those for whom it isn't, they can set the limit during
the build.
We already had this for the plain parser, so TinyCBOR wouldn't cause a
stack overflow in case of a malformed stream (intentionally or not) when
simply parsing and advancing over the stream. This same protection
wasn't applied to the content converting from CBOR to JSON.
Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
Coverity complains that, when CBOR_ENCODER_WRITE_FUNCTION is defined,
enc.data is read in cbor_encoder_create_map() when
cbor_encoder_init_writer() didn't write to it.
While 'data' is merely copied in cbor_encoder_create_map(), Coverity
is right, though: reading an uninitialized value is UB.
Fix by setting data.writer to nullptr (abstracting the difference
between C and C++ behind a new macro).
`Q_ASSERT()` disappears in release mode, leading Clang to print a static
analysis warning about an impossible condition:
```
tst_parser.cpp:251:16: warning: variable 'err' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
251 | } else if (ourType == CborTextStringType) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tst_parser.cpp:263:12: note: uninitialized use occurs here
263 | return err;
| ^~~
tst_parser.cpp:251:12: note: remove the 'if' if its condition is always true
```
`Q_UNREACHABLE()` becomes a plain `__builtin_unreachable()` in release
mode.
Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
The /* fallthrough */ comment isn't always handled, causing some
compilers to complain about falling through.
Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
Follow the same "pattern" as for the rest of the #if directive: first
test if a macro is defined, then test the value. Otherwise the code
triggers a -Wundef warning (e.g. when building in C++).
Homebrew no longer carries precompiled versions of Qt for macos-11, so
switch to macos-13, the last on x86 CPUs (so we can still run
Valgrind). It's also going away after the end of June.
Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
We don't support this any more.
```
../../src/cbor.h:255:69: error: '_Bool' is a C99 extension [-Werror,-Wc99-extensions]
CBOR_INLINE_API CborError cbor_encode_boolean(CborEncoder *encoder, bool value)
^
```
Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
It doesn't know -Wdiscarded-qualifiers. Reported by @pjonsson on #247.
I really need to switch to CMake - #242
Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
The Qt uses test batching and potentially encoder/data.cpp
and parser/data.cpp can end up in the same translation unit.
This can be problematic as they declare symbols with the
same names.
Change both files to use internal linkage in order to avoid
symbols clashing.
Qt is defaulting to QT_NO_FOREACH these days, so make sure we
integrate nicely with downstream and fix the single Q_FOREACH/foreach
user, in tst_encoder.cpp.
Unfortunately, the container's initialization code doesn't exactly
lend itself to making the container const (not even IILE
(Immediately-Invoked Lambda Expression) would help here, due to the
interdependency with `len`), so the idiomatic solution would be to use
std::as_const()/qAsConst().
The former is available from C++17, which we don't require, yet, and
the latter is not available under QT_NO_AS_CONST (the default for Qt
these days), so grab the nettle and implement a t17::as_const() that
switches between a manual implementation of std::as_const and the real
thing, depending on __cpp_lib_as_const. The `t17` here mimicks the qNN
(q20::remove_cvref_t/q23::forward_like/etc) mechanism used in Qt
itself for backports, with s/q/t/ because ... _T_inyCbor.
The t17 implementation is local to tst_encoder.cpp, but can easily be
extracted into a separate header once more users emerge.
Recently I was working on a project where I had to define CBOR_PARSER_ADVANCE_BYTES_FUNCTION and CBOR_PARSER_TRANSFER_STRING_FUNCTION macros. Unfortunately the library doesn't include any external config file and building system I am using provides only possibility to add simple preprocessor definitions (like definition = value) on building script level. Adding macros is not possible. For this reason I am asking if we can add above change - if I add simple definition CBOR_EXTERNAL_CFG by build system then cbor will require cbor_cfg.h file where I can add needed macros.
Thiago Macieira