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

Stack based buffer overflow while parsing JSON file #601

Closed
cve-reporting opened this Issue Jul 11, 2018 · 2 comments

Comments

Projects
None yet
2 participants
@cve-reporting
Copy link

cve-reporting commented Jul 11, 2018

Function push() that implements stack push operation does not check whether new elements are still within allocated buffer.
It returns error only before ending the function, but this value is never checked in JSON parser, so the next push operations are not blocked.

contiki-ng/os/lib/json/jsonparse.h:41:

	#define JSONPARSE_MAX_DEPTH 10

Declaration of the buffer: contiki-ng/os/lib/json/jsonparse.h:44-55

	struct jsonparse_state {
	...
		char stack[JSONPARSE_MAX_DEPTH];
	};

Function push(): contiki-ng/os/lib/json/jsonparse.c:37-44

	static int
	push(struct jsonparse_state *state, char c)
	{
	  state->stack[state->depth] = c;
	  state->depth++;
	  state->vtype = 0;
	  return state->depth < JSONPARSE_MAX_DEPTH;
	}

Sample usage of push(): contiki-ng/os/lib/json/jsonparse.c:114

	int
	jsonparse_next(struct jsonparse_state *state)
	{
	  char c;
	  char s;
	  char v;

	  skip_ws(state);
	  c = state->json[state->pos];
	  s = jsonparse_get_type(state);
	  v = state->vtype;
	  state->pos++;

	  switch(c) {
	  case '{':
	    if((s == 0 && v == 0) || s == '[' || s == ':') {
	      push(state, c);

Vulnerability is not likely to be used for Remote Code Execution exploit, because it allows to only put two types of characters to this stack: left curly bracket ({), left square bracket ([), so the risk was reduced (Confidentiality: None, Integrity: Low, Scope: Unchanged).
However it is quite easy to overwrite the function return address on stack or stack canary, which leads to crash of the application (via segmentation fault) and can be used by attacker for Denial of Service attacks.
Additionally, the risk of this issue is also reduced (Attack Vector:Local) because attacker would need to parse malicious JSON, however it is quite possible when using JSON input data in IoT application.

Proposed CVSS score:

CVSS:3.0/AV:L/AC:L/PR:N/UI:N/S:U/C:N/I:L/A:H (6.8 - Medium)

Following JSON will trigger crash:

	{"a": { "a": { "a": { "a": { "a": { "a": { "a": { "a": { "a": { "a": { "a": { "a": {

or using square brackets:

	{"a": [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[

Mitigation :
Function push() should not allow to add new elements to the stack after reaching the limit (JSONPARSE_MAX_DEPTH).
Functions using the push() function should check the returned value.

Crash details using Address Sanitizer:

==5275==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffd92c1c648 at pc 0x0000004042f1 bp 0x7ffd92c1c5a0 sp 0x7ffd92c1c590
WRITE of size 1 at 0x7ffd92c1c648 thread T0
    #0 0x4042f0 in push contiki-ng/os/lib/json/jsonparse.c:40
    #1 0x4042f0 in jsonparse_next contiki-ng/os/lib/json/jsonparse.c:176
    #2 0x401903 in main contiki-ng/os/lib/json/test_json.c:82
    #3 0x7f355825782f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #4 0x401d68 in _start (contiki-ng/os/lib/json/test_json_dbg.exe+0x401d68)

Address 0x7ffd92c1c648 is located in stack of thread T0 at offset 72 in frame
    #0 0x4016af in main contiki-ng/os/lib/json/test_json.c:58

  This frame has 1 object(s):
    [32, 72) 'state' <== Memory access at offset 72 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow contiki-ng/os/lib/json/jsonparse.c:40 push
Shadow bytes around the buggy address:
  0x10003257b870: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10003257b880: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10003257b890: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10003257b8a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10003257b8b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x10003257b8c0: f1 f1 f1 f1 00 00 00 00 00[f4]f4 f4 f3 f3 f3 f3
  0x10003257b8d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10003257b8e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10003257b8f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10003257b900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10003257b910: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
==5275==ABORTING
@simonduq

This comment has been minimized.

Copy link
Member

simonduq commented Oct 15, 2018

A fix was proposed in #703

@simonduq simonduq self-assigned this Oct 19, 2018

@simonduq

This comment has been minimized.

Copy link
Member

simonduq commented Oct 25, 2018

Closed with #703

@simonduq simonduq closed this Oct 25, 2018

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