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. quickjs/quickjs-libc.c

   Line 1568 in 6e2e68f

   JS_NewClass(JS_GetRuntime(ctx), js_std_file_class_id, &js_std_file_class);

2. quickjs/quickjs-libc.c

   Line 3768 in 6e2e68f

   JS_NewClass(JS_GetRuntime(ctx), js_worker_class_id, &js_worker_class);

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().