Njs: exotic prototypes and its getters

[drsm]

>> Object.create([]).length
=================================================================
==31207==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x604000003af4 at pc 0x55afbb8f34f2 bp 0x7ffef43cbf50 sp 0x7ffef43cbf40
READ of size 4 at 0x604000003af4 thread T0
    #0 0x55afbb8f34f1 in njs_array_prototype_length njs/njs_array.c:450
    #1 0x55afbb8cb100 in njs_value_property njs/njs_vm.c:2993
    #2 0x55afbb8c060a in njs_vmcode_property_get njs/njs_vm.c:481
    #3 0x55afbb8bf50b in njs_vmcode_interpreter njs/njs_vm.c:158
    #4 0x55afbb8be05b in njs_vm_start njs/njs.c:594
    #5 0x55afbb8af705 in njs_process_script njs/njs_shell.c:701
    #6 0x55afbb8ae427 in njs_interactive_shell njs/njs_shell.c:448
    #7 0x55afbb8ad84c in main njs/njs_shell.c:255
    #8 0x7fe4dc9c8b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
    #9 0x55afbb8ad149 in _start (/home/drsm/work/njs/build/njs+0x25149)

0x604000003af4 is located 4 bytes to the right of 32-byte region [0x604000003ad0,0x604000003af0)
allocated by thread T0 here:
    #0 0x7fe4dd6be7a0 in posix_memalign (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdf7a0)
    #1 0x55afbb965b26 in nxt_memalign nxt/nxt_malloc.c:26
    #2 0x55afbb8bb4af in njs_align njs/njs.c:41
    #3 0x55afbb8b654a in nxt_mp_alloc_large nxt/nxt_mp.c:594
    #4 0x55afbb8b620b in nxt_mp_alloc nxt/nxt_mp.c:310
    #5 0x55afbb8e4894 in njs_object_alloc njs/njs_object.c:36
    #6 0x55afbb8e8902 in njs_object_create njs/njs_object.c:839
    #7 0x55afbb90d9bd in njs_function_native_call njs/njs_function.c:533
    #8 0x55afbb8c7dc6 in njs_vmcode_function_call njs/njs_vm.c:2018
    #9 0x55afbb8bf50b in njs_vmcode_interpreter njs/njs_vm.c:158
    #10 0x55afbb8be05b in njs_vm_start njs/njs.c:594
    #11 0x55afbb8af705 in njs_process_script njs/njs_shell.c:701
    #12 0x55afbb8ae427 in njs_interactive_shell njs/njs_shell.c:448
    #13 0x55afbb8ad84c in main njs/njs_shell.c:255
    #14 0x7fe4dc9c8b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)

SUMMARY: AddressSanitizer: heap-buffer-overflow njs/njs_array.c:450 in njs_array_prototype_length
Shadow bytes around the buggy address:
  0x0c087fff8700: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd fa
  0x0c087fff8710: fa fa 00 00 00 00 00 fa fa fa fd fd fd fd fd fa
  0x0c087fff8720: fa fa 00 00 00 00 00 fa fa fa 00 00 00 00 00 fa
  0x0c087fff8730: fa fa 00 00 00 00 00 fa fa fa 00 00 00 00 00 fa
  0x0c087fff8740: fa fa 00 00 00 00 00 fa fa fa 00 00 00 00 00 fa
=>0x0c087fff8750: fa fa 00 00 00 00 00 fa fa fa 00 00 00 00[fa]fa
  0x0c087fff8760: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c087fff8770: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c087fff8780: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c087fff8790: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c087fff87a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
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
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  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
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==31207==ABORTING

also:

>> Object.create(function(a,b,c) {}).length
=================================================================
==31275==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x604000003db7 at pc 0x5592dfec05b6 bp 0x7ffc175a5840 sp 0x7ffc175a5830
READ of size 1 at 0x604000003db7 thread T0
    #0 0x5592dfec05b5 in njs_function_prototype_length njs/njs_function.c:854
    #1 0x5592dfe7d100 in njs_value_property njs/njs_vm.c:2993
    #2 0x5592dfe7260a in njs_vmcode_property_get njs/njs_vm.c:481
    #3 0x5592dfe7150b in njs_vmcode_interpreter njs/njs_vm.c:158
    #4 0x5592dfe7005b in njs_vm_start njs/njs.c:594
    #5 0x5592dfe61705 in njs_process_script njs/njs_shell.c:701
    #6 0x5592dfe60427 in njs_interactive_shell njs/njs_shell.c:448
    #7 0x5592dfe5f84c in main njs/njs_shell.c:255
    #8 0x7fac016fdb96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
    #9 0x5592dfe5f149 in _start (/home/drsm/work/njs/build/njs+0x25149)

[xeioex]

The issue is: in njs "length" getter/setter is in Array.prototype. Whereas according to ES6 it is simply the property of an instance. The same with function.

[drsm]

@xeioex maybe "fix" this like that done in String.length?
Object.create(Object('asdf')).length does not trigger ASAN.

[xeioex]

@drsm
I think the proper solution is to move "length" out of a prototype, and simply add it to array object during its creation. It would automatically solve many issues with 'exotic' length properties.

Alternatively, as it is in String.prototype.length the getter and setter should check the type of this object. But it looks like a half-solution.

[xeioex]

@drsm

Take a look
https://gist.github.com/xeioex/7e4e51ca6eecd57572d18486bd769b8a

The patch splits instance from prototype properties.
It simplifies njs_object_enumerate(). It also related to #106. Arrow function will have their own instance shared hash.

[drsm]

@xeioex
the patch works fine,
but property handlers are still invoked on the target object,
and there is no proto lookup:

>> var o = Object.create([1,2,3])
undefined
>> o[1]
2
>> o.length
0
>> o.length = 5
5
>> o.length
5

and

>> Object.create(function(a,b,c) {}).length
Segmentation fault

so, we have to either made handlers polymorphic or fix an object its invoked on

[xeioex]

@drsm

but property handlers are still invoked on the target object,

yes. This patch is not a solution for the issue. Rather this is a prerequisite.

[xeioex]

@drsm

but property handlers are still invoked on the target object,

I am thinking what is the proper way to do it. According to.

9.1.8 [[Get]] (P, Receiver)
When the [[Get]] internal method of O is called with property key P and ECMAScript language value Receiver the following steps are taken:

Assert: IsPropertyKey(P) is true.
Let desc be O.[[GetOwnProperty]](P).
ReturnIfAbrupt(desc).
If desc is undefined, then
Let parent be O.[[GetPrototypeOf]]().
ReturnIfAbrupt(parent).
If parent is null, return undefined.
Return parent.[[Get]](P, Receiver).
If IsDataDescriptor(desc) is true, return desc.[[Value]].
Otherwise, IsAccessorDescriptor(desc) must be true so, let getter be desc.[[Get]].
If getter is undefined, return undefined.
Return Call(getter, Receiver).

Call(getter, Receiver)

So, for example for var a = [1,2]; var o = Object.create(a); o.length, is not it means that length getter should receive the Object o, not Array a?

[drsm]

So, for example for var a = [1,2]; var o = Object.create(a); o.length, is not it means that length getter should receive the Object o, not Array a?

in this case, we are go to step 4.d Return parent.[[Get]](P, Receiver)..

the step 8 is for:

> var f = function() {};
undefined
> Object.defineProperty(f, 'test', { get: function() { return typeof this; }})
[Function: f]
> f.test
'function'
> var o = Object.create(f)
undefined
> o.test
'object'

Array.length is the data property, but its setter has side effects, so Array is an exotic object in terms of javascript.
and Receiver value is used only for accessor properties.

[xeioex]

@drsm

in this case, we are go to step 4.d Return parent.[Get].

yes, but still original Receiver will be passed to the getter.

[xeioex]

@drsm

Take a look at the following patch
https://gist.github.com/xeioex/37f201cc97f84ad7c2f5b663edafefb8

[drsm]

@xeioex

Looks good, thanks!