Return value of JS_NewClass not checked in js_std_init/js_os_init

[webmaster128]

Hihi,

I am trying to debug crashes of quickjs in case of low memory settings. One of the problems I hit was

Assertion failed: (class_id < rt->class_count), function JS_SetClassProto, file quickjs.c, line 2208.

(which is here)

Turns out that this assertion is hit because the two JS_NewClass calls do not check the return value:

1. https://github.com/bellard/quickjs/blob/6e2e68fd0896957f92eb6c242a2e048c1ef3cae0/quickjs-libc.c#L1568
2. https://github.com/bellard/quickjs/blob/6e2e68fd0896957f92eb6c242a2e048c1ef3cae0/quickjs-libc.c#L3768

Do I understand this is a missing error check? Should I provide a PR to fix this? Is the error signature of JS_NewClass the same as JS_NewClass1?

Thanks!

[bellard]

Unfortunately there are many places in the quickjs initialization where failures are not tested, so fixing JS_NewClass() is useless if the rest is not fixed as well.

[webmaster128]

I see. We also found a handful of different cases during our testing so far.

But wouldn't it be worth fixing them one-by-one to get more visibility of the remaining cases and eventually solve them all?

in the quickjs initialization

Are you referring to the js_std_init/js_os_init functions? Or something else?

Our high level motivation is to run untrusted JS code with a memory limit. Right now it is possible to crash the application by sending JS code that is too big. This then crashes before the execution starts.

[bellard]

I am referring all the init functions, in particular JS_NewContext(), js_std_init and js_os_init().