Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cbor_value_copy_byte_string causes stack corruption #194

Closed
StefanHri opened this issue Jan 11, 2021 · 3 comments
Closed

cbor_value_copy_byte_string causes stack corruption #194

StefanHri opened this issue Jan 11, 2021 · 3 comments

Comments

@StefanHri
Copy link

The documentation of the function cbor_value_copy_byte_string says:

If the buffer is large enough, this function will insert a null byte after the last copied byte, to facilitate manipulation of null-terminated strings. 

However, the null byte is appended without checking if the size of buffer:

CborError _cbor_value_copy_string(const CborValue *value, void *buffer,
                                 size_t *buflen, CborValue *next)
{
    bool copied_all;
    CborError err = iterate_string_chunks(value, (char*)buffer, buflen, &copied_all, next,
                                          buffer ? (IterateFunction) value->parser->d->cpy : iterate_noop);
    if (err) {
        return err;
    }

    if (!copied_all) {
        return CborErrorOutOfMemory;
    }

    if (buffer) {
        *((uint8_t *)buffer + *buflen) = '\0';
    }

    return CborNoError;
}

This causes that one byte belonging to some other variable is overwritten.

@thiagomacieira
Copy link
Member

I can't find this version of the code. What version of TinyCBOR are you using?

The function _cbor_value_copy_string was introduced in commit ff130bc and did not look like that. The braces in the function also do not match the coding style I use.

thiagomacieira added a commit to thiagomacieira/tinycbor that referenced this issue Jan 12, 2021
Or don't terminate, as the case may be.

Relates to intel#194.

Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
thiagomacieira added a commit to thiagomacieira/tinycbor that referenced this issue Jan 12, 2021
Or don't terminate, as the case may be.

Relates to intel#194.

Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
thiagomacieira pushed a commit to thiagomacieira/tinycbor that referenced this issue Jan 12, 2021
Matching commit ee9bc24:
parser: add a test that the string copy functions properly terminate

Or don't terminate, as the case may be.

Relates to intel#194.

Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
thiagomacieira pushed a commit to thiagomacieira/tinycbor that referenced this issue Jan 12, 2021
Matching commit ee9bc24:
parser: add a test that the string copy functions properly terminate

Or don't terminate, as the case may be.

Relates to intel#194.

Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
thiagomacieira pushed a commit to thiagomacieira/tinycbor that referenced this issue Jan 12, 2021
Matching commit ee9bc24:
parser: add a test that the string copy functions properly terminate

Or don't terminate, as the case may be.

Relates to intel#194.

Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
thiagomacieira pushed a commit to thiagomacieira/tinycbor that referenced this issue Jan 12, 2021
Matching commit ee9bc24:
parser: add a test that the string copy functions properly terminate

Or don't terminate, as the case may be.

Relates to intel#194.

Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
@StefanHri
Copy link
Author

@thiagomacieira
Copy link
Member

Please report to them. Their copy is modified.

thiagomacieira added a commit that referenced this issue Jan 13, 2021
Or don't terminate, as the case may be.

Relates to #194.

Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
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

No branches or pull requests

2 participants