Skip to content

Parser: fix reading it->extra on big endian when bytesNeeded == 1#1

Merged
thiagomacieira merged 1 commit into
thiagomacieira:devfrom
mitya57:dev
Apr 13, 2019
Merged

Parser: fix reading it->extra on big endian when bytesNeeded == 1#1
thiagomacieira merged 1 commit into
thiagomacieira:devfrom
mitya57:dev

Conversation

@mitya57

@mitya57 mitya57 commented Apr 11, 2019

Copy link
Copy Markdown

&it->extra points to the wrong byte of uint16_t on big endian systems.

By using a temporary one-byte value we ensure that the code works fine on both big and little endian systems.

Results of running tests in tests/parser/ on s390x before this fix:

Totals: 2037 passed, 293 failed, 0 skipped, 0 blacklisted, 221ms

After this fix:

Totals: 2330 passed, 0 failed, 0 skipped, 0 blacklisted, 254ms

@thiagomacieira

Copy link
Copy Markdown
Owner

Hello Dmitry

Thank you for the patch. I'm staring at it and not sure I get it. Was the problem on quantities >= 256 or was it on 1-byte quantities? The 16-bit codepath is identical to what it was, so it must be the 8-bit one, right?

PS: can you do a git commit --amend -s and then re-push, so we get the commit with a Signed-off-by?

@thiagomacieira

Copy link
Copy Markdown
Owner

Ah, I see the commit message says it's bytesNeeded == 1. Got it.

Can you update with your sign off?

Signed-off-by: Dmitry Shachnev <mitya57@gmail.com>
@mitya57

mitya57 commented Apr 12, 2019

Copy link
Copy Markdown
Author

Can you update with your sign off?

Done!

@thiagomacieira thiagomacieira merged commit d70c649 into thiagomacieira:dev Apr 13, 2019
qtprojectorg pushed a commit to qt/qtbase that referenced this pull request Apr 25, 2019
Original commit from thiagomacieira/tinycbor#1

Change-Id: I194d3f37471a49788a7bfffd1594ef5db19465fd
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
thiagomacieira pushed a commit that referenced this pull request Mar 14, 2025
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
        intel#3 0x560b654f267d in value_to_json /tinycbor/src/cbortojson.c:674:19
        intel#4 0x560b654f34c2 in array_to_json /tinycbor/src/cbortojson.c:545:25
        intel#5 0x560b654f2085 in value_to_json /tinycbor/src/cbortojson.c:627:19
        intel#6 0x560b654f1baf in cbor_value_to_json_advance /tinycbor/src/cbortojson.c:816:12
        intel#7 0x560b654ea928 in dumpFile /tinycbor/tools/cbordump/cbordump.c:76:19
        intel#8 0x560b654ead2b in main /tinycbor/tools/cbordump/cbordump.c:149:9
        intel#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: e072bc1 ("CBOR-to-JSON: do properly escape JSON strings")
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants