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

Double-free when printing cJSON_Raw if valuestring==NULL #241

Closed
projectgus opened this issue Feb 7, 2018 · 4 comments
Closed

Double-free when printing cJSON_Raw if valuestring==NULL #241

projectgus opened this issue Feb 7, 2018 · 4 comments

Comments

@projectgus
Copy link

projectgus commented Feb 7, 2018

I was taking a look at the cJSON codebase on behalf of a customer, and I a spot that I wanted to check when printing cJSON_Raw values:

  • If item->valuestring is NULL, the print_value() implementation may deallocate the write buffer and then return. However, at least some callers of print_value() (for example print()) will also deallocate this buffer on failure. This looks like a potential double-free.

I haven't had time to be verify this for certain, or write a test, but I thought I would report it while I had it in front of me. Seems like it's probably an unlikely path to hit in real code.

EDIT: Previous version of this issue had two points. For one, I missed that the memcpy included the terminating byte.

@projectgus projectgus changed the title Memory issues when printing cJSON_Raw Double-free when printing cJSON_Raw if valuestring==NULL Feb 7, 2018
@FSMaxB
Copy link
Collaborator

FSMaxB commented Feb 7, 2018

You are right. All callers of print_value deallocate the buffer on failure, except in cJSON_PrintPreallocated, where there are no allocations anyway.

I will fix this and make a new release immediately.

@FSMaxB
Copy link
Collaborator

FSMaxB commented Feb 7, 2018

Thanks a lot for reviewing cJSON!

@FSMaxB FSMaxB closed this as completed in d514bb8 Feb 7, 2018
@FSMaxB
Copy link
Collaborator

FSMaxB commented Feb 7, 2018

1.7.3 has been released with a fix.

@projectgus
Copy link
Author

Wow, that's great. Thanks for the quick response @FSMaxB !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants