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

static cJSON_bool print_value(const cJSON * const item, printbuffer * const output_buffer) #492

Closed
GitHub-LHD opened this issue Aug 4, 2020 · 15 comments

Comments

@GitHub-LHD
Copy link

The print_value() very absurd!!!
eg:
(1) if output_buffer->buffer is global, the function is very danger!!!
(2) struct{
int flag;
char buf[];
}
if output_buffer->buffer point to buf[] and buf[] realloced, oh mg god!!!

@Alanscut
Copy link
Collaborator

Alanscut commented Aug 5, 2020

Hi @mengxiangren , function print_value and struct printbuffer is not exposed to users, and the memory allocation of the print_buffer is performed inside the static function. Where do you find that the output_buffer->buffer becomes global?

@GitHub-LHD
Copy link
Author

GitHub-LHD commented Aug 5, 2020 via email

@GitHub-LHD
Copy link
Author

Explame one:

static buf[100];
cJSON root;

cJSON_PrintPreallocated(root, buf, 100, 1)
{
printbuffer p = { 0, 0, 0, 0, 0, 0, { 0, 0, 0 } };
...
print_value(item, &p)
{
...
ensure(output_buffer, 1000); //if reallocte buf, it is error
....
}
}

Explame two:

static struct fa{
int flag;
char *buf[];
};
static struct fa *flexible_array;
cJSON root;

flexible_array = molloc(sizeof(struct fa) + 100);

cJSON_PrintPreallocated(root, flexible_array->buf, 100, 1)
{
printbuffer p = { 0, 0, 0, 0, 0, 0, { 0, 0, 0 } };
...
print_value(item, &p)
{
...
ensure(output_buffer, 1000); //if reallocte buf, it is error
....
}
}

printf("%s", flexible_array->buf);//if reallocte buf, it is error

@Alanscut
Copy link
Collaborator

Alanscut commented Aug 5, 2020

realloc the output_buffer->buffer won't report error, realloc the buf just change the size of memory block pointed by buf. Have you ever tested your snippet? what error has your program reported? I compiled your example one with valgrind, and it didn't detect any memory errors.
image
image

@GitHub-LHD
Copy link
Author

realloc the output_buffer->buffer won't report error, realloc the buf just change the size of memory block pointed by buf. Have you ever tested your snippet? what error has your program reported? I compiled your example one with valgrind, and it didn't detect any memory errors.
image
image

Use you code, now set buf[2],then run again
static char buf[2]
cJSON_PrintPreallocated(root, buf, 2, 1);

@Alanscut
Copy link
Collaborator

Alanscut commented Aug 5, 2020

then the cJSON_PrintPreallocated will return 0, namely failed to print root to buf, still no error.

@Alanscut
Copy link
Collaborator

Alanscut commented Aug 5, 2020

I suggest you test it in your server and post the error logs, in case I mishandle something.

@GitHub-LHD
Copy link
Author

then the cJSON_PrintPreallocated will return 0, namely failed to print root to buf, still no error.

I means, the buf(static) shouldn't reallocate, becase the buf is global

@GitHub-LHD
Copy link
Author

then the cJSON_PrintPreallocated will return 0, namely failed to print root to buf, still no error.

I means, the buf(static) shouldn't reallocate, becase the buf is global

if static buf[100], the realloc(50), it is right
if static buf[50], the realloc(100), it is error

@Alanscut
Copy link
Collaborator

Alanscut commented Aug 5, 2020

I suggest you test it in your server and post the error logs, in case I mishandle something.

are you test example two?
yes, example two certainly reported an error, because cJSON_PrintPreallocated can't accept a char * buf[], but a char *

static struct fa{
int flag;
char *buf[]; // this is a char **
};

@GitHub-LHD
Copy link
Author

I suggest you test it in your server and post the error logs, in case I mishandle something.

are you test example two?
yes, example two certainly reported an error, because cJSON_PrintPreallocated can't accept a char * buf[], but a char *

static struct fa{
int flag;
char *buf[]; // this is a char **
};

sorry,it should
static struct fa{
int flag;
char buf[];
};

@GitHub-LHD
Copy link
Author

I suggest you test it in your server and post the error logs, in case I mishandle something.

are you test example two?
yes, example two certainly reported an error, because cJSON_PrintPreallocated can't accept a char * buf[], but a char *

static struct fa{
int flag;
char *buf[]; // this is a char **
};

sorry,it should
static struct fa{
int flag;
char buf[];
};

struct fa is contiguous memory address, if reallocate fa.buf, then struct fa will not contiguous memory address

@Alanscut
Copy link
Collaborator

Alanscut commented Aug 5, 2020

realloc function won't invoked in the cJSON_PrintPreallocated function.

cJSON/cJSON.c

Lines 1286 to 1303 in cf97c6f

CJSON_PUBLIC(cJSON_bool) cJSON_PrintPreallocated(cJSON *item, char *buffer, const int length, const cJSON_bool format)
{
printbuffer p = { 0, 0, 0, 0, 0, 0, { 0, 0, 0 } };
if ((length < 0) || (buffer == NULL))
{
return false;
}
p.buffer = (unsigned char*)buffer;
p.length = (size_t)length;
p.offset = 0;
p.noalloc = true;
p.format = format;
p.hooks = global_hooks;
return print_value(item, &p);
}

if the length of buffer is smaller than needed, then it will return NULL in line464.

cJSON/cJSON.c

Lines 434 to 465 in cf97c6f

/* realloc printbuffer if necessary to have at least "needed" bytes more */
static unsigned char* ensure(printbuffer * const p, size_t needed)
{
unsigned char *newbuffer = NULL;
size_t newsize = 0;
if ((p == NULL) || (p->buffer == NULL))
{
return NULL;
}
if ((p->length > 0) && (p->offset >= p->length))
{
/* make sure that offset is valid */
return NULL;
}
if (needed > INT_MAX)
{
/* sizes bigger than INT_MAX are currently not supported */
return NULL;
}
needed += p->offset + 1;
if (needed <= p->length)
{
return p->buffer + p->offset;
}
if (p->noalloc) {
return NULL;
}

@GitHub-LHD
Copy link
Author

realloc function won't invoked in the cJSON_PrintPreallocated function.

cJSON/cJSON.c

Lines 1286 to 1303 in cf97c6f

CJSON_PUBLIC(cJSON_bool) cJSON_PrintPreallocated(cJSON *item, char *buffer, const int length, const cJSON_bool format)
{
printbuffer p = { 0, 0, 0, 0, 0, 0, { 0, 0, 0 } };
if ((length < 0) || (buffer == NULL))
{
return false;
}
p.buffer = (unsigned char*)buffer;
p.length = (size_t)length;
p.offset = 0;
p.noalloc = true;
p.format = format;
p.hooks = global_hooks;
return print_value(item, &p);
}

if the length of buffer is smaller than needed, then it will return NULL in line464.

cJSON/cJSON.c

Lines 434 to 465 in cf97c6f

/* realloc printbuffer if necessary to have at least "needed" bytes more */
static unsigned char* ensure(printbuffer * const p, size_t needed)
{
unsigned char *newbuffer = NULL;
size_t newsize = 0;
if ((p == NULL) || (p->buffer == NULL))
{
return NULL;
}
if ((p->length > 0) && (p->offset >= p->length))
{
/* make sure that offset is valid */
return NULL;
}
if (needed > INT_MAX)
{
/* sizes bigger than INT_MAX are currently not supported */
return NULL;
}
needed += p->offset + 1;
if (needed <= p->length)
{
return p->buffer + p->offset;
}
if (p->noalloc) {
return NULL;
}

Sorry, i misunderstood

@GitHub-LHD
Copy link
Author

realloc function won't invoked in the cJSON_PrintPreallocated function.

cJSON/cJSON.c

Lines 1286 to 1303 in cf97c6f

CJSON_PUBLIC(cJSON_bool) cJSON_PrintPreallocated(cJSON *item, char *buffer, const int length, const cJSON_bool format)
{
printbuffer p = { 0, 0, 0, 0, 0, 0, { 0, 0, 0 } };
if ((length < 0) || (buffer == NULL))
{
return false;
}
p.buffer = (unsigned char*)buffer;
p.length = (size_t)length;
p.offset = 0;
p.noalloc = true;
p.format = format;
p.hooks = global_hooks;
return print_value(item, &p);
}

if the length of buffer is smaller than needed, then it will return NULL in line464.

cJSON/cJSON.c

Lines 434 to 465 in cf97c6f

/* realloc printbuffer if necessary to have at least "needed" bytes more */
static unsigned char* ensure(printbuffer * const p, size_t needed)
{
unsigned char *newbuffer = NULL;
size_t newsize = 0;
if ((p == NULL) || (p->buffer == NULL))
{
return NULL;
}
if ((p->length > 0) && (p->offset >= p->length))
{
/* make sure that offset is valid */
return NULL;
}
if (needed > INT_MAX)
{
/* sizes bigger than INT_MAX are currently not supported */
return NULL;
}
needed += p->offset + 1;
if (needed <= p->length)
{
return p->buffer + p->offset;
}
if (p->noalloc) {
return NULL;
}

I have another question, in print_value()

static cJSON_bool print_value(const cJSON * const item, printbuffer * const output_buffer)
{
...
output = ensure(output_buffer, 5); //5 = strlen("null") + strlen("");
if (output == NULL)
{
return false;
}
strcpy((char*)output, "null");
...
}
static unsigned char* ensure(printbuffer * const p, size_t needed)
{
...
needed += p->offset + 1; //why does needed + 1?
if (needed <= p->length) //if length= 5, print_value() will return false, but it should have return "null"
{ //so, it should be "output = ensure(output_buffer, 4)"?
return p->buffer + p->offset;
}
if (p->noalloc)
{
return NULL;
}
...
}

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