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

Add Symbol key support to WeakMap #56

Closed
wants to merge 1 commit into from
Closed

Add Symbol key support to WeakMap #56

wants to merge 1 commit into from

Conversation

saghul
Copy link
Contributor

@saghul saghul commented Nov 13, 2023

No description provided.

@saghul
Copy link
Contributor Author

saghul commented Nov 13, 2023

Locally I get a segfault which looks like a UAF, digging...

@saghul
Copy link
Contributor Author

saghul commented Nov 13, 2023

Process 12397 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x10)
    frame #0: 0x0000000100061f7c qjs`js_map_finalizer [inlined] delete_weak_ref(rt=<unavailable>, mr=0x0000600002608660) at quickjs.c:43559:15 [opt]
   43556	    p = JS_VALUE_GET_OBJ(mr->key);
   43557	    pmr = &p->first_weak_ref;
   43558	    for(;;) {
-> 43559	        mr1 = *pmr;
   43560	        assert(mr1 != NULL);
   43561	        if (mr1 == mr)
   43562	            break;
Target 0: (qjs) stopped.
warning: qjs was compiled with optimization - stepping may behave oddly; variables may not be available.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x10)
  * frame #0: 0x0000000100061f7c qjs`js_map_finalizer [inlined] delete_weak_ref(rt=<unavailable>, mr=0x0000600002608660) at quickjs.c:43559:15 [opt]
    frame #1: 0x0000000100061f70 qjs`js_map_finalizer(rt=0x0000000100504080, val=<unavailable>) at quickjs.c:43797:21 [opt]
    frame #2: 0x0000000100063628 qjs`free_object(rt=0x0000000100504080, p=0x000060000210dfe0) at quickjs.c:5192:9 [opt]
    frame #3: 0x0000000100030550 qjs`JS_RunGC [inlined] free_gc_object(rt=0x0000000100504080, gp=<unavailable>) at quickjs.c:5212:9 [opt]
    frame #4: 0x0000000100030538 qjs`JS_RunGC [inlined] gc_free_cycles(rt=0x0000000100504080) at quickjs.c:5532:13 [opt]
    frame #5: 0x00000001000304e8 qjs`JS_RunGC(rt=0x0000000100504080) at quickjs.c:5562:5 [opt]
    frame #6: 0x000000010002f850 qjs`JS_FreeRuntime(rt=0x0000000100504080) at quickjs.c:1760:5 [opt]
    frame #7: 0x0000000100004388 qjs`main(argc=2, argv=0x000000016fdff438) at qjs.c:488:5 [opt]
    frame #8: 0x000000010011d08c dyld`start + 520
(lldb) p *mr
(JSMapRecord) $1 = {
  ref_count = 1
  empty = NO
  map = 0x0000600000c103f0
  next_weak_ref = 0xfffffffffffffff8
  link = {
    prev = 0x0000600000c103f8
    next = 0x0000600000c103f8
  }
  hash_link = {
    prev = 0x0000600000010040
    next = 0x0000600000010040
  }
  key = {
    u = (int32 = 2136512, float64 = 5.2150169614339714E-310, ptr = 0x00006000002099c0)
    tag = -8
  }
  value = {
    u = (int32 = 42, float64 = 2.0750757125332355E-322, ptr = 0x000000000000002a)
    tag = 0
  }
}
(lldb) print *(JSObject*)0x00006000002099c0
(JSObject) $2 = {
   = {
    header = {
      ref_count = 1
      gc_obj_type = JS_GC_OBJ_TYPE_VAR_REF
      mark = '\0'
      dummy1 = '\0'
      dummy2 = 0
      link = {
        prev = 0x00000229c0000000
        next = 0x0000000000666572
      }
    }
     = {
      __gc_ref_count = 1
      __gc_mark = '\x03'
      extensible = '\0'
      free_mark = '\0'
      is_exotic = '\0'
      fast_array = '\0'
      is_constructor = '\0'
      is_uncatchable_error = '\0'
      tmp_mark = '\0'
      is_HTMLDDA = '\0'
      class_id = 0
    }
  }
  shape = NULL
  prop = 0x0000beadde8d99e0
  first_weak_ref = 0x00006000026000d4
  u = {
    opaque = 0x000000000000002a
    bound_function = 0x000000000000002a
    c_function_data_record = 0x000000000000002a
    for_in_iterator = 0x000000000000002a
    array_buffer = 0x000000000000002a
    typed_array = 0x000000000000002a
    map_state = 0x000000000000002a
    map_iterator_data = 0x000000000000002a
    array_iterator_data = 0x000000000000002a
    regexp_string_iterator_data = 0x000000000000002a
    generator_data = 0x000000000000002a
    proxy_data = 0x000000000000002a
    promise_data = 0x000000000000002a
    promise_function_data = 0x000000000000002a
    async_function_data = 0x000000000000002a
    async_from_sync_iterator_data = 0x000000000000002a
    async_generator_data = 0x000000000000002a
    func = {
      function_bytecode = 0x000000000000002a
      var_refs = NULL
      home_object = 0x0000beadde8d9a00
    }
    cfunc = {
      realm = 0x000000000000002a
      c_function = {
        generic = 0x0000000000000000
        generic_magic = 0x0000000000000000
        constructor = 0x0000000000000000
        constructor_magic = 0x0000000000000000
        constructor_or_func = 0x0000000000000000
        f_f = 0x0000000000000000
        f_f_f = 0x0000000000000000
        getter = 0x0000000000000000
        setter = 0x0000000000000000
        getter_magic = 0x0000000000000000
        setter_magic = 0x0000000000000000
        iterator_next = 0x0000000000000000
      }
      length = '\0'
      cproto = '\x9a'
      magic = -8563
    }
    array = {
      u1 = {
        size = 42
        typed_array = 0x000000000000002a
      }
      u = {
        values = NULL
        ptr = 0x0000000000000000
        int8_ptr = 0x0000000000000000
        uint8_ptr = 0x0000000000000000
        int16_ptr = 0x0000000000000000
        uint16_ptr = 0x0000000000000000
        int32_ptr = 0x0000000000000000
        uint32_ptr = 0x0000000000000000
        int64_ptr = 0x0000000000000000
        uint64_ptr = 0x0000000000000000
        float_ptr = 0x0000000000000000
        double_ptr = 0x0000000000000000
      }
      count = 3733821952
    }
    regexp = {
      pattern = 0x000000000000002a
      bytecode = NULL
    }
    object_data = {
      u = (int32 = 42, float64 = 2.0750757125332355E-322, ptr = 0x000000000000002a)
      tag = 0
    }
  }
}
(lldb) print *(JSMapRecord*)0x00006000026000d4
(JSMapRecord) $3 = {
  ref_count = 8
  empty = NO
  map = NULL
  next_weak_ref = NULL
  link = {
    prev = NULL
    next = NULL
  }
  hash_link = {
    prev = NULL
    next = NULL
  }
  key = {
    u = (int32 = 0, float64 = 0, ptr = 0x0000000000000000)
    tag = 0
  }
  value = {
    u = (int32 = 0, float64 = -1.0793736765470836E+139, ptr = 0xdccd012000000000)
    tag = -272700358476115
  }
}

Looks like the map record referenced by the Symbol key is corrupted. This does not happen with an object as a key.

@saghul
Copy link
Contributor Author

saghul commented Nov 13, 2023

==2515642==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x5643e952548e in delete_weak_ref /home/saghul/src/quickjs/quickjs.c:43577:13
    #1 0x5643e952548e in js_map_finalizer /home/saghul/src/quickjs/quickjs.c:43813:21
    #2 0x5643e9529cac in free_object /home/saghul/src/quickjs/quickjs.c:5192:9
    #3 0x5643e9529cac in free_gc_object /home/saghul/src/quickjs/quickjs.c:5212:9
    #4 0x5643e9490c2f in gc_free_cycles /home/saghul/src/quickjs/quickjs.c:5532:13
    #5 0x5643e9490c2f in JS_RunGC /home/saghul/src/quickjs/quickjs.c:5562:5
    #6 0x5643e948716b in JS_FreeRuntime /home/saghul/src/quickjs/quickjs.c:1760:5
    #7 0x5643e946d29f in main /home/saghul/src/quickjs/qjs.c:488:5
    #8 0x7fd61943fccf  (/usr/lib/libc.so.6+0x27ccf) (BuildId: 8bfe03f6bf9b6a6e2591babd0bbc266837d8f658)
    #9 0x7fd61943fd89 in __libc_start_main (/usr/lib/libc.so.6+0x27d89) (BuildId: 8bfe03f6bf9b6a6e2591babd0bbc266837d8f658)
    #10 0x5643e93d9454 in _start (/home/saghul/src/quickjs/build/qjs+0x28454) (BuildId: 8212d58eed0a421cc5bb4d6a3b67b21b715afaf3)

@saghul
Copy link
Contributor Author

saghul commented Nov 13, 2023

Ah I think I know what's going on. Symbols seem to be implemented as atoms with some flags, not as objects. So, by accessing it as an object we are just tinkering around with the surrounding memory I think!

@bnoordhuis do you reckon I'm on the right track here? If so, not sure how to continue from here 😅

@bnoordhuis
Copy link
Contributor

So, a Symbol is not exactly an atom but a JSValue where the .ptr field points to a JSAtomStruct, and that's an alias for JSString (a refcounted object.)

It seems like it should Just Work(TM). Let me try locally.

@saghul saghul closed this Nov 14, 2023
@saghul saghul deleted the weakmap-symbol branch November 30, 2023 08:56
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

Successfully merging this pull request may close these issues.

2 participants