cabo / cn-cbor Goto Github PK

View Code? Open in Web Editor NEW

110.0 110.0 79.0 108 KB

A constrained node implementation of CBOR in C

License: MIT License

Shell 0.09% CMake 28.89% C 71.02%

cn-cbor's People

Contributors

Stargazers

Watchers

cn-cbor's Issues

coveralls

Add support for coveralls. Depends on #17.

Parser does not respect tag 24 - It does not defer the decoding of data with this tag

On of the things I was looking at for the COSE specification was the use of tag 24 rather than doing a binary wrapper around some data so that it would be transported and could be extracted intact rather than requiring re-encoding. However the parser does not stop parsing when a tag 24 is found but continues all the way down. This should be fixed.

Do epsilon-based float serializer

Need a way to indicate whether 64-bit integers are desired or not

Currently, struct cn_cbor keeps uint/sint as "unsigned long".
This is for applications that don't really need 64-bit integers.
However, some do; there should be a configuration option to enable this.

Make float optional (#ifdef)

Strings not null-terminated after decode

Hello, everyone. This may be intended behavior (although I can't see why), but it appears that if you stick a string inside a map and encode it, that string will not be null terminated after decoding. Here is an example of what I mean:

cn_cbor_mapput_string(inner_map, "username", cbor_username, &ctx, NULL);
cn_cbor_mapput_string(inner_map, "message", cbor_message, &ctx, NULL);

Assume cbor_username and cbor_message are of type const char *. Also assume inner_map is a map. I then stick inner_map inside another map and encode it. On decoding, I recover the strings.

const cn_cbor * cbor_username = cn_cbor_mapget_string(inner_map, "username");
const cn_cbor * cbor_message = cn_cbor_mapget_string(inner_map, "message");

The field cbor_username->v.str is not null-terminated. You can still recover the length of the string through cbor_username->length. This is also true of cbor_message.

Any thoughts? Is this a known issue? Is this intended?

Define a field in struct cn_cbor.v that is of type uint8_t *

Currently if you use a cn_cbor structure to build a new one of type bytes, then you need to do a cast in order to go from char * to uint8_t *. Creating the new field would allow for bytes to be unsigned rather than signed when accessed.

is this project still maintained?

@cabo I'm doing some packaging for conan to use this project as dependency of https://github.com/openthread/ot-commissioner

now my question: is this still maintained? I've been doing MRs to https://github.com/jimsch/cn-cbor/tree/complete though normally debian and conan want upstream urls...

Could you make a statement on this?

Consider indicating the size of floats in the API in the cb->length field

    case AI_2:
#ifndef CBOR_NO_FLOAT
      cb->type = CN_CBOR_DOUBLE;
      cb->v.dbl = decode_half(val);
      cb->length = 2; // Tiny Float, especially nice for serializing binary fractions
#else /*  CBOR_NO_FLOAT */
      CN_CBOR_FAIL(CN_CBOR_ERR_FLOAT_NOT_SUPPORTED);
#endif /*  CBOR_NO_FLOAT */
      break;
    case AI_4:
#ifndef CBOR_NO_FLOAT
      cb->type = CN_CBOR_DOUBLE;
      u32.u = val;
      cb->v.dbl = u32.f;
      cb->length = 4; // Short Float, good enough for most constrained network purposes
#else /*  CBOR_NO_FLOAT */
      CN_CBOR_FAIL(CN_CBOR_ERR_FLOAT_NOT_SUPPORTED);
#endif /*  CBOR_NO_FLOAT */
      break;
    case AI_8:
#ifndef CBOR_NO_FLOAT
      cb->type = CN_CBOR_DOUBLE;
      u64.u = val;
      cb->v.dbl = u64.d;
      cb->length = 8; // Long Float, no excuses
#else /*  CBOR_NO_FLOAT */
      CN_CBOR_FAIL(CN_CBOR_ERR_FLOAT_NOT_SUPPORTED);
#endif /*  CBOR_NO_FLOAT */
      break;
    default: cb->v.uint = val;

Wrong use of index variable after increment in cbor_test.c

In parse_normalize test, the index variable i is increased as a side effect in ASSERT_TRUE(). The following CTEST_LOG therefore accesses the wrong array index and finally violates the array's boundaries during the last iteration.

This bug is fixed in the code for PR 20 but I can open a new PR for this if required.

Coverage for float code

The block here isn't currently hit by the tests.

cn_cbor_free is either overactive or needs to be even more overactive

If you free a node which is not the root of a tree, will cause all of the trailing siblings and parents to be freed as well. But it will not free the previous sibling nodes and there children.

Either it should fail if it is not the root of the tree, or it should free all of the nodes in the tree instead of just some of them.

ssize_t is from POSIX and not from the C standard

The serialization code is using ssize_t rather than size_t so that it can mark errors with a negative value. The problem with this is that ssize_t is not part of standard C and therefore is not supported by all compilers.

Should we change this or should we just have non-supporting compilers re-define ssize_t appropriately?

Note that the Microsoft compiler is one of the ones which does not support this type.

Make it build for windows

This is partly done, but there are still some thing to be finished.

If the system builds a DLL rather than a static library, the symbols to be exported from the DLL need to be identified. There are two ways of doing this:

Create a file which lists the points to be exported.
Use a compiler directive to tag which points are to be exported. (See http://www.cmake.org/Wiki/BuildingWinDLL for a fuller description of what this involves.)

The second method makes it clear what is happening, but makes the code be messier so you end up with code the following in cn-cbor.h

MYLIB_EXPORT
cn_cbor* cn_cbor_map_create(CBOR_CONTEXT_COMMA cn_cbor_errback *errp);

The first method means that people who only develop on Linux systems will have no clue that there is one additional step that needs to be done if a new entry point to the library is added.

I can easily do either method, and I don't think that I currently have a preference. There is no need to mark things as imports from a dll in order to link to a library. But we don't currently process cn-cbor.h to produce an internal and external version.

const behavior makes life difficult

The returning of const objects from a number of calls is making life difficult. It is not clear that this is even the correct behavior. It makes sense to say that items are const if one is looking at a tree which has been parsed. But if one is looking at a tree that one is currently building then this is not so clear.

Consider the following code:

const cn_cbor * pRecipients = cn_cbor_mapget_int(cose->m_message.m_cbor, 10);
if (pRecipients == NULL) {
cn_cbor * p = cn_cbor_array_create(NULL);
cn_cbor_mapput_int(cose->m_message.m_cbor, 10, p, NULL);
}
cn_cbor_array_append(pRecipients, pobj->m_encrypt.m_message.m_cbor, NULL);

In this code, I am retrieving an array from the structure I am building, creating the array if it does not already exist, and then appending a new element to the array. However, the fact that it wants to return a const cbor element means that I need to do a cast someplace which loses the const tag. It is not clear to me that this is what should be happening. Rather it might make more sense to have people apply const to locals in their own code when items are retrieved instead.

Add diagnostic format serializer

Should be pretty straightforward to re-use visit(). Support indented/pretty formatted as well.

Ready for release?

I think we're getting close to a 1.0 release. We can keep working after a release...

cn-cbor not portable for MCU that doesn't support alignment

I would use cn-cbor on a ARM Cortex-M0, and I'm facing a problem with some dependencies.

Here https://github.com/cabo/cn-cbor/blob/master/src/cn-cbor.c#L16 and here https://github.com/cabo/cn-cbor/blob/master/src/cn-encoder.c#L11, arpa/inet.h is included to use ntohl. Problem is that I don't want to include arpa/inet.h, which would be a waste of flash space.

I want then propose a modification : https://gist.github.com/ks156/9b8d1a1d97ec1b38f91e

I can send a pull request if you want, but before, this code must be reviewed. I only started to work with cn-cbor few days ago and I don't have enough experience to know if these changes can have unexpected side effects. Basically, there's no change as long as CBOR_COMPAT is not defined. But if someone can confirm, it would be nice.

BR,

Paul

Overflow detection on -18446744073709551616 broken

Need a serializer option to get size of buffer needed

Currently one has to allocate a buffer and then serialize into it. There is nothing that gives you any idea of how big the buffer needs to be.

One reasonable way to give this functionality would be to all passing in a NULL for the destination buffer and then return the size of the needed buffer.

Not convinced that doubles are in network byte order

  u64.u = val;
  cb->v.dbl = u64.d;

seems a little fishy to me.

Look out for size-based calculations that break on integer overflow

e.g., (ws->offset + (sz) >= ws->size) (in ensure_writable)
This is unlikely to occur in normal practice, but could be used in an attack.

Failing to build tests when CMAKE_BUILD_TYPE MinSizeRel on Linux

First discovered when building spudlib on Ubuntu.

OS: Linux ubuntu 3.19.0-16-generic #16-Ubuntu SMP Thu Apr 30 16:09:58 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux
CC: cc (Ubuntu 4.9.2-10ubuntu13) 4.9.2

Diff to reproduce bug (Sorry about my lack of cmake understanding but
cmake .. -DCMAKE_BUILD_TYPE=MinRelSize -Dcoveralls=OFF -Dcoveralls_send=OFF -Dverbose=ON did not provoke the bug)

-if (NOT CMAKE_BUILD_TYPE)

if ( optimize )
set ( CMAKE_BUILD_TYPE MinSizeRel )
set ( coveralls OFF )
set ( coveralls_send OFF )
else ()
set ( CMAKE_BUILD_TYPE Debug )
endif ()
-endif()
+set ( CMAKE_BUILD_TYPE MinSizeRel )
+set ( coveralls OFF )
+set ( coveralls_send OFF )

+#if (NOT CMAKE_BUILD_TYPE)
+# if ( optimize )
+# set ( CMAKE_BUILD_TYPE MinSizeRel )
+# set ( coveralls OFF )
+# set ( coveralls_send OFF )
+# else ()
+# set ( CMAKE_BUILD_TYPE Debug )
+# endif ()
+#endif()

Gives following compile warnings:
d /home/palmarti/development/cn-cbor/build/test && /usr/bin/cc -Os -DNDEBUG -I/home/palmarti/development/cn-cbor/test/../include -I/home/palmarti/development/cn-cbor/src/../include -std=gnu99 -Wall -Wextra -pedantic -Werror -o CMakeFiles/cbor_test.dir/cbor_test.c.o -c /home/palmarti/development/cn-cbor/test/cbor_test.c
/home/palmarti/development/cn-cbor/test/cbor_test.c: In function ‘_ctest_cbor_parse_normalize_run’:
/home/palmarti/development/cn-cbor/test/cbor_test.c:168:9: error: iteration 18ul invokes undefined behavior [-Werror=aggressive-loop-optimizations]
CTEST_LOG("%s: %s", tests[i], cn_cbor_error_str[err.err]);
^
/home/palmarti/development/cn-cbor/test/cbor_test.c:163:5: note: containing loop
for (i=0; i<sizeof(tests)/sizeof(char); ) {
^
cc1: all warnings being treated as errors
test/CMakeFiles/cbor_test.dir/build.make:57: recipe for target 'test/CMakeFiles/cbor_test.dir/cbor_test.c.o' failed
make[2]: *_* [test/CMakeFiles/cbor_test.dir/cbor_test.c.o] Error 1

Fuzz testing

Hello,

I am looking into this library to use for serialization of data. I have used fuzz testing for testing similar libraries to find bugs. Based on the test case in cbor_test.c i wrote a simple fuzz testing suite. Using this i have detected some possible crashes. However, i don't know much of the API and how it is intended to be used. If i use it wring this might not be an issue.

Also, i noticed that if i try to encode the decoded data, i need a bigger buffer: cbor_libfuzzer.c#L33. Any ideas why?

Example output:

$ make libfuzzer_asan
$ ./libfuzzer_asan.out seed
INFO: Seed: 2458114154
INFO: Loaded 1 modules   (183 inline 8-bit counters): 183 [0x55a776557260, 0x55a776557317),
INFO: Loaded 1 PC tables (183 PCs): 183 [0x55a776557318,0x55a776557e88),
INFO:       41 files found in seed/
INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes
INFO: seed corpus: files: 41 min: 1b max: 11b total: 146b rss: 26Mb
=================================================================
==6182==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000001f31 at pc 0x55a7765121a8 bp 0x7ffcc1033c70 sp 0x7ffcc1033c68
READ of size 8 at 0x602000001f31 thread T0
    #0 0x55a7765121a7 in decode_item /Users/simonj/Development/aa/aadcp/cn-cbor/fuzz-test/../src/cn-cbor.c:155:14
    #1 0x55a7765110fd in cn_cbor_decode /Users/simonj/Development/aa/aadcp/cn-cbor/fuzz-test/../src/cn-cbor.c:264:9
    #2 0x55a7765109eb in LLVMFuzzerTestOneInput /Users/simonj/Development/aa/aadcp/cn-cbor/fuzz-test/cbor_libfuzzer.c:28:10
    #3 0x55a7763ddfa5 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/Users/simonj/Development/aa/aadcp/cn-cbor/fuzz-test/libfuzzer_asan.out+0x38fa5)
    #4 0x55a7763e078d in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) (/Users/simonj/Development/aa/aadcp/cn-cbor/fuzz-test/libfuzzer_asan.out+0x3b78d)
    #5 0x55a7763e344f in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, fuzzer::fuzzer_allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) (/Users/simonj/Development/aa/aadcp/cn-cbor/fuzz-test/libfuzzer_asan.out+0x3e44f)
    #6 0x55a7763e4c42 in fuzzer::Fuzzer::Loop(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, fuzzer::fuzzer_allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) (/Users/simonj/Development/aa/aadcp/cn-cbor/fuzz-test/libfuzzer_asan.out+0x3fc42)
    #7 0x55a7763d5b42 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/Users/simonj/Development/aa/aadcp/cn-cbor/fuzz-test/libfuzzer_asan.out+0x30b42)
    #8 0x55a7763c9233 in main (/Users/simonj/Development/aa/aadcp/cn-cbor/fuzz-test/libfuzzer_asan.out+0x24233)
    #9 0x7f4d2318d222 in __libc_start_main (/usr/lib/libc.so.6+0x24222)
    #10 0x55a7763c926d in _start (/Users/simonj/Development/aa/aadcp/cn-cbor/fuzz-test/libfuzzer_asan.out+0x2426d)

0x602000001f35 is located 0 bytes to the right of 5-byte region [0x602000001f30,0x602000001f35)
allocated by thread T0 here:
    #0 0x55a7764d3419 in __interceptor_malloc (/Users/simonj/Development/aa/aadcp/cn-cbor/fuzz-test/libfuzzer_asan.out+0x12e419)
    #1 0x7f4d2358e5fc in operator new(unsigned long) /build/gcc/src/gcc/libstdc++-v3/libsupc++/new_op.cc:50:40
    #2 0x55a7763e078d in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) (/Users/simonj/Development/aa/aadcp/cn-cbor/fuzz-test/libfuzzer_asan.out+0x3b78d)
    #3 0x55a7763e344f in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, fuzzer::fuzzer_allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) (/Users/simonj/Development/aa/aadcp/cn-cbor/fuzz-test/libfuzzer_asan.out+0x3e44f)
    #4 0x55a7763e4c42 in fuzzer::Fuzzer::Loop(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, fuzzer::fuzzer_allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) (/Users/simonj/Development/aa/aadcp/cn-cbor/fuzz-test/libfuzzer_asan.out+0x3fc42)
    #5 0x55a7763d5b42 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/Users/simonj/Development/aa/aadcp/cn-cbor/fuzz-test/libfuzzer_asan.out+0x30b42)
    #6 0x55a7763c9233 in main (/Users/simonj/Development/aa/aadcp/cn-cbor/fuzz-test/libfuzzer_asan.out+0x24233)
    #7 0x7f4d2318d222 in __libc_start_main (/usr/lib/libc.so.6+0x24222)

SUMMARY: AddressSanitizer: heap-buffer-overflow /Users/simonj/Development/aa/aadcp/cn-cbor/fuzz-test/../src/cn-cbor.c:155:14 in decode_item
Shadow bytes around the buggy address:
  0x0c047fff8390: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fd fa
  0x0c047fff83a0: fa fa 04 fa fa fa 00 fa fa fa fd fa fa fa fd fa
  0x0c047fff83b0: fa fa fd fd fa fa fd fd fa fa fd fa fa fa fd fa
  0x0c047fff83c0: fa fa fd fa fa fa fd fa fa fa 04 fa fa fa 00 04
  0x0c047fff83d0: fa fa fd fa fa fa fd fa fa fa fd fd fa fa fd fd
=>0x0c047fff83e0: fa fa 05 fa fa fa[05]fa fa fa fa fa fa fa fa fa
  0x0c047fff83f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8400: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8410: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8420: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8430: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==6182==ABORTING
MS: 0 ; base unit: 0000000000000000000000000000000000000000
0xfa,0x47,0x80,0x0,0x0,
\xfaG\x80\x00\x00
artifact_prefix='./'; Test unit written to ./crash-258cde6e8feef6766b423ad40acdcf94a84f0cc6
Base64: +keAAAA=

$ xxd -p crash-258cde6e8feef6766b423ad40acdcf94a84f0cc6
fa47800000
$ make debug
$ ./debug < crash-258cde6e8feef6766b423ad40acdcf94a84f0cc6
=================================================================
==6185==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000000011 at pc 0x5643bbaba4f3 bp 0x7fff5047b410 sp 0x7fff5047b400
READ of size 8 at 0x602000000011 thread T0
    #0 0x5643bbaba4f2 in decode_item ../src/cn-cbor.c:155
    #1 0x5643bbabb26f in cn_cbor_decode ../src/cn-cbor.c:264
    #2 0x5643bbab9448 in LLVMFuzzerTestOneInput /Users/simonj/Development/aa/aadcp/cn-cbor/fuzz-test/cbor_libfuzzer.c:28
    #3 0x5643bbab990e in main /Users/simonj/Development/aa/aadcp/cn-cbor/fuzz-test/cbor_libfuzzer.c:86
    #4 0x7f75e9e5a222 in __libc_start_main (/usr/lib/libc.so.6+0x24222)
    #5 0x5643bbab927d in _start (/Users/simonj/Development/aa/aadcp/cn-cbor/fuzz-test/debug.out+0x227d)

0x602000000015 is located 0 bytes to the right of 5-byte region [0x602000000010,0x602000000015)
allocated by thread T0 here:
    #0 0x7f75ea0ec019 in __interceptor_malloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:86
    #1 0x5643bbab98bd in main /Users/simonj/Development/aa/aadcp/cn-cbor/fuzz-test/cbor_libfuzzer.c:79
    #2 0x7f75e9e5a222 in __libc_start_main (/usr/lib/libc.so.6+0x24222)

SUMMARY: AddressSanitizer: heap-buffer-overflow ../src/cn-cbor.c:155 in decode_item
Shadow bytes around the buggy address:
  0x0c047fff7fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c047fff8000: fa fa[05]fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8010: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8020: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8050: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==6185==ABORTING

travis

Add support for continuous integration with travis-ci: http://docs.travis-ci.com/user/languages/c/

Suggest renaming cn_cbor_index to cn_cbor_array_index

Having the word array in the function name would make it clear that this is what it is used for. Trying to remember the name without that help is harder given that a search for array does not find it. Note that all of the map functions have map in them. i.e. cn_cbor_mapget_int.

For complete consistency with maps it really out to be cn_cbor_arrayget_index, but that seems not to be correct.

Serializer

Starting a serializer feels like a good amount of design work that I'd want to talk to you about first.

cabo / cn-cbor Goto Github PK

cn-cbor's People

Contributors

Stargazers

Watchers

Forkers

cn-cbor's Issues

Recommend Projects

Recommend Topics

Recommend Org