details: https://hg.nginx.org/njs/rev/a41cd80e2f3a
branches:
changeset: 1599:a41cd80e2f3a
user: Dmitry Volyntsev <xeioex@nginx.com>
date: Mon Feb 01 02:16:51 2021 +0100
description:
Fixed safe mode bypass in Function constructor.

This closes #367 pull request on Github.

Thanks bux.patryk@gmail.com for prodding it.

diffstat:

src/njs_function.c | 82 ++++++++++++++++++++++++++++-------------------
src/test/njs_unit_test.c | 25 ++++++++++++++
2 files changed, 74 insertions(+), 33 deletions(-)

diffs (150 lines):

diff -r 8d4b7b3ae73d -r a41cd80e2f3a src/njs_function.c
--- a/src/njs_function.c Mon Feb 01 16:13:10 2021 +0000
+++ b/src/njs_function.c Mon Feb 01 02:16:51 2021 +0100
@@ -869,41 +869,31 @@ static njs_int_t
njs_function_constructor(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
njs_index_t unused)
{
- njs_chb_t chain;
- njs_int_t ret;
- njs_str_t str, file;
- njs_uint_t i;
- njs_value_t *body;
- njs_lexer_t lexer;
- njs_parser_t *parser;
- njs_vm_code_t *code;
- njs_function_t *function;
- njs_generator_t generator;
- njs_parser_scope_t *scope;
- njs_function_lambda_t *lambda;
+ njs_chb_t chain;
+ njs_int_t ret;
+ njs_str_t str, file;
+ njs_uint_t i;
+ njs_lexer_t lexer;
+ njs_parser_t *parser;
+ njs_vm_code_t *code;
+ njs_function_t *function;
+ njs_generator_t generator;
+ njs_parser_node_t *node;
+ njs_parser_scope_t *scope;
+ njs_function_lambda_t *lambda;
+ const njs_token_type_t *type;

- if (!vm->options.unsafe) {
- body = njs_argument(args, nargs - 1);
- ret = njs_value_to_string(vm, body, body);
- if (njs_slow_path(ret != NJS_OK)) {
- return ret;
- }
-
- njs_string_get(body, &str);
+ static const njs_token_type_t safe_ast[] = {
+ NJS_TOKEN_END,
+ NJS_TOKEN_FUNCTION_EXPRESSION,
+ NJS_TOKEN_STATEMENT,
+ NJS_TOKEN_RETURN,
+ NJS_TOKEN_THIS,
+ NJS_TOKEN_ILLEGAL
+ };

- /*
- * Safe mode exception:
- * "(new Function('return this'))" is often used to get
- * the global object in a portable way.
- */
-
- if (str.length != njs_length("return this")
- || memcmp(str.start, "return this", 11) != 0)
- {
- njs_type_error(vm, "function constructor is disabled"
- " in \"safe\" mode");
- return NJS_ERROR;
- }
+ if (!vm->options.unsafe && nargs != 2) {
+ goto fail;
}

njs_chb_init(&chain, vm->mem_pool);
@@ -960,6 +950,27 @@ njs_function_constructor(njs_vm_t *vm, n
return ret;
}

+ if (!vm->options.unsafe) {
+ /*
+ * Safe mode exception:
+ * "(new Function('return this'))" is often used to get
+ * the global object in a portable way.
+ */
+
+ node = parser->node;
+ type = &safe_ast[0];
+
+ for (; *type != NJS_TOKEN_ILLEGAL; type++, node = node->right) {
+ if (node == NULL || node->left != NULL) {
+ goto fail;
+ }
+
+ if (node->token_type != *type) {
+ goto fail;
+ }
+ }
+ }
+
scope = parser->scope;

ret = njs_variables_copy(vm, &scope->variables, vm->variables_hash);
@@ -998,6 +1009,11 @@ njs_function_constructor(njs_vm_t *vm, n
njs_set_function(&vm->retval, function);

return NJS_OK;
+
+fail:
+
+ njs_type_error(vm, "function constructor is disabled in \"safe\" mode");
+ return NJS_ERROR;
}


diff -r 8d4b7b3ae73d -r a41cd80e2f3a src/test/njs_unit_test.c
--- a/src/test/njs_unit_test.c Mon Feb 01 16:13:10 2021 +0000
+++ b/src/test/njs_unit_test.c Mon Feb 01 02:16:51 2021 +0100
@@ -19518,6 +19518,25 @@ static njs_unit_test_t njs_test[] =
};


+static njs_unit_test_t njs_safe_test[] =
+{
+ { njs_str("(new Function('return this'))() === globalThis"),
+ njs_str("true") },
+
+ { njs_str("(new Function('return this;'))() === globalThis"),
+ njs_str("true") },
+
+ { njs_str("(new Function('return this '))() === globalThis"),
+ njs_str("true") },
+
+ { njs_str("(new Function('return thi'))()"),
+ njs_str("TypeError: function constructor is disabled in \"safe\" mode") },
+
+ { njs_str("(new Function('){return 1337})//', 'return this'))()"),
+ njs_str("TypeError: function constructor is disabled in \"safe\" mode") },
+};
+
+
static njs_unit_test_t njs_denormals_test[] =
{
{ njs_str("2.2250738585072014e-308"),
@@ -21777,6 +21796,12 @@ static njs_test_suite_t njs_suites[] =
njs_nitems(njs_test),
njs_unit_test },

+ { njs_str("safe script"),
+ { .repeat = 1},
+ njs_safe_test,
+ njs_nitems(njs_safe_test),
+ njs_unit_test },
+
{ njs_str("denormals"),
{ .repeat = 1, .unsafe = 1 },
njs_denormals_test,
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel