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

An unsafe operation is found in the S2J_STRUCT_GET_string_ELEMENT function #13

Closed
marckwei opened this issue Nov 18, 2020 · 5 comments
Closed

Comments

@marckwei
Copy link

struct2json

Vulnerability Analysis

An unsafe operation is found in the S2J_STRUCT_GET_string_ELEMENTfunction. The strcpy function is used to copy JSON->value to the struct, which may cause overflow when JSON->value is longer than structure defined array size.

img

POC

#include "s2j.h" 
#include <stdint.h> 
#include <stdio.h> 
 
typedef struct { 
  char name[8]; 
} Hometown; 
 
static void *json_to_struct(cJSON* json_obj) { 
  /* create Hometown structure object */ 
  s2j_create_struct_obj(struct_hometown, Hometown); 
 
  /* deserialize data to Hometown structure object. */ 
  s2j_struct_get_basic_element(struct_hometown, json_obj,string, name); 
  return struct_hometown; 
} 
 
 
int main(void) { 
  cJSON * json=NULL; 
 
  json=cJSON_CreateObject(); 
 
  cJSON_AddStringToObject(json, "name", "ABCDEFGHIJKLMNOPQRSTUVWXYZABCDEFGHIJKLMNOPQRSTUVWXYZABCDEFGHIJKLMNOPQRSTUVWXYZ"); 
   
  Hometown * test=json_to_struct(json); 
 
  printf("that\'s fine!%zu",sizeof(test)); 
 
 
  return 0; 
}

Run:

img

Suggestion

Use strncpy instead of strcpy to control the length of JSON->value

#define S2J_STRUCT_GET_string_ELEMENT(to_struct, from_json, _element) \ 
  json_temp = cJSON_GetObjectItem(from_json, #_element); \ 
  if (json_temp) strncpy((to_struct)->_element, json_temp->valuestring,sizeof((to_struct)->_element));

After modification:

img

@libradoge
Copy link
Contributor

字符串的话,是否要修改成:sizeof是否要-1:

#define S2J_STRUCT_GET_string_ELEMENT(to_struct, from_json, _element) \ 
  json_temp = cJSON_GetObjectItem(from_json, #_element); \ 
  if (json_temp) strncpy((to_struct)->_element, json_temp->valuestring,sizeof((to_struct)->_element) -1 );

@abergmann
Copy link

CVE-2020-29203 was assigned to this issue.

@armink
Copy link
Owner

armink commented Dec 28, 2020

Thanks for your feedback, can you submit a PR for it?

@libradoge
Copy link
Contributor

Thanks for your feedback, can you submit a PR for it?

It's my first time to submit a PR, so please check my work carefully...And thank you for giving me this chance!

@armink
Copy link
Owner

armink commented Dec 29, 2020

Thanks for your feedback, can you submit a PR for it?

It's my first time to submit a PR, so please check my work carefully...And thank you for giving me this chance!

Good job.

Thank you for your contribution, PR has been merged

@armink armink closed this as completed Dec 29, 2020
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

4 participants