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

Overwriting terminated null character #3001

Closed
smorzhov opened this issue Sep 7, 2021 · 4 comments
Closed

Overwriting terminated null character #3001

smorzhov opened this issue Sep 7, 2021 · 4 comments
Labels
solution: invalid the issue is not related to the library

Comments

@smorzhov
Copy link

smorzhov commented Sep 7, 2021

json/src/json.hpp

Line 4684 in ec7a1d8

result[pos] = '\\';

I think it's better to check here the size of result and do something like:

if (result.size() > pos) {
    // overwrite trailing null character
    result[pos] = '\\';
}
@nlohmann
Copy link
Owner

nlohmann commented Sep 7, 2021

I'm not sure if I understand your proposal. Here is the original code for reference:

if (c >= 0x00 and c <= 0x1f)
{
    // print character c as \uxxxx
    sprintf(&result[pos + 1], "u%04x", int(c));
    pos += 6;
    // overwrite trailing null character
    result[pos] = '\\';
}

The string result has already the correct size and contains entirely of slashes. Therefore, we can start writing the u1234 output at pos+1 to have \u1234. sprintf null-terminates the string, so result[pos] is a null byte. As later escapings rely on slashes in result, we overwrite that null byte with a slash.

@smorzhov
Copy link
Author

smorzhov commented Sep 7, 2021

Let's see what's going on if we have "\x01" string. extra_space for that string will return 5. So result in escape_string will be of size 6 (1 + 5). And at the end you are doing result[6] = '\\';. Isn't this an attempt to write out of memory?

@smorzhov smorzhov closed this as completed Sep 7, 2021
@smorzhov smorzhov reopened this Sep 7, 2021
@smorzhov
Copy link
Author

smorzhov commented Sep 7, 2021

Sorry, accidentally closed the issue

@nlohmann
Copy link
Owner

nlohmann commented Sep 7, 2021

I just realized that the code you refer to is nearly 6 years old and has since been changed several times. See https://github.com/nlohmann/json/blob/develop/include/nlohmann/detail/output/serializer.hpp#L449 for the current implementation which looks much different to the previous one.

@nlohmann nlohmann closed this as completed Sep 7, 2021
@nlohmann nlohmann added the solution: invalid the issue is not related to the library label Sep 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
solution: invalid the issue is not related to the library
Projects
None yet
Development

No branches or pull requests

2 participants