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

cJSON_Minify cross-border read&write 1 #337

Closed
bigric3 opened this issue Feb 21, 2019 · 5 comments

Comments

Projects
None yet
3 participants
@bigric3
Copy link

commented Feb 21, 2019

https://github.com/DaveGamble/cJSON/blob/master/cJSON.c : 2678
can bypass '\x00',bug can be trigger with json string buffer that end with '\x00' or not

       else if (*json == '\"')
        {
            /* string literals, which are \" sensitive. */
            *into++ = (unsigned char)*json++;
            while (*json && (*json != '\"'))
            {
                if (*json == '\\')
                {
                    *into++ = (unsigned char)*json++;
                }
                *into++ = (unsigned char)*json++;
            }
            *into++ = (unsigned char)*json++;
        }

test demo, compile at ubuntu x86, cause stackoverflow:

#include <stdlib.h>  
#include "cJSON.h"  
  
int main (int argc, const char * argv[])  
{  

    char testdata[8] = {'\t','\t','\t','\t','\"','\\','n','n'};

    cJSON_Minify(testdata); // vuln function
    
    printf("target:%s\n",testdata);
    
    return 0;  
}  

@FSMaxB FSMaxB added the question label Apr 12, 2019

@FSMaxB

This comment has been minimized.

Copy link
Collaborator

commented Apr 12, 2019

You must never pass a non-null terminated string into cJSON. Without the \0 at the end cJSON does not know where the string ends and will do a buffer overread either until there is some random 0 somewhere in memory.

Your code would be correct like this:

#include <stdlib.h>
#include "cJSON.h"

int main (int argc, const char * argv[])
{
    char testdata[9] = {'\t','\t','\t','\t','\"','\\','n','n', '\0'};

    cJSON_Minify(testdata);

    printf("target:%s\n",testdata);

    return 0;  
}
@FSMaxB

This comment has been minimized.

Copy link
Collaborator

commented Apr 12, 2019

What actually happens is not a stackoverflow btw., it is a buffer overflow on the stack though.

@FSMaxB FSMaxB closed this Apr 12, 2019

@bigric3

This comment has been minimized.

Copy link
Author

commented Apr 13, 2019

yes sir,this question‘s root cause same as issues 338,but 2 different places.
bug can be trigger with json string buffer that end with '\x00'
below can bypass '\x00', cause cross border read/write

`
else if (*json == '"')
{

        /* string literals, which are \" sensitive. */
        *into++ = (unsigned char)*json++;
        while (*json && (*json != '\"'))
        {
            if (*json == '\\')
            {
                *into++ = (unsigned char)*json++;
            }
            *into++ = (unsigned char)*json++;
        }
        *into++ = (unsigned char)*json++;
    }

`

should edit as below
`
else if (*json == '"')
{

        /* string literals, which are \" sensitive. */
        *into++ = (unsigned char)*json++;
        while (*json && (*json != '\"'))
        {
            if (*json == '\\')
            {
                *into++ = (unsigned char)*json++;
            }
            // here add1
           if(*json == '\x00')
                 // return or break;
            *into++ = (unsigned char)*json++;
           // here add2
            if(*json == '\x00')
                 // return or break;
        }
        *into++ = (unsigned char)*json++;
    }

`

@FSMaxB

This comment has been minimized.

Copy link
Collaborator

commented Apr 15, 2019

cJSON_Minify has been fixed in release 1.7.11

@carnil

This comment has been minimized.

Copy link

commented May 9, 2019

CVE-2019-11834 was assigned for this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.