Skip to content

Commit aa358e1

Browse files
xeioexxeioex
committed
Fixed Function constructor template injection.
The Function constructor uses a `(function(<args>) {<body>})` template to construct new functions. This approach was vulnerable to template injection where malicious code could close the function body early. This fixes issue #921.
1 parent f1aa752 commit aa358e1

2 files changed

Lines changed: 28 additions & 12 deletions

File tree

‎src/njs_function.c‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1049,7 +1049,7 @@ njs_function_constructor(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
10491049
}
10501050
}
10511051

1052-
njs_chb_append_literal(&chain, "){");
1052+
njs_chb_append_literal(&chain, "\n){\n");
10531053

10541054
if (nargs > 1) {
10551055
ret = njs_value_to_chain(vm, &chain, njs_argument(args, nargs - 1));
@@ -1058,7 +1058,7 @@ njs_function_constructor(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
10581058
}
10591059
}
10601060

1061-
njs_chb_append_literal(&chain, "})");
1061+
njs_chb_append_literal(&chain, "\n})");
10621062

10631063
ret = njs_chb_join(&chain, &str);
10641064
if (njs_slow_path(ret != NJS_OK)) {
@@ -1125,7 +1125,15 @@ njs_function_constructor(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
11251125

11261126
njs_chb_destroy(&chain);
11271127

1128-
lambda = ((njs_vmcode_function_t *) generator.code_start)->lambda;
1128+
if ((code->end - code->start)
1129+
!= (sizeof(njs_vmcode_function_t) + sizeof(njs_vmcode_return_t))
1130+
|| ((njs_vmcode_generic_t *) code->start)->code != NJS_VMCODE_FUNCTION)
1131+
{
1132+
njs_syntax_error(vm, "single function literal required");
1133+
return NJS_ERROR;
1134+
}
1135+
1136+
lambda = ((njs_vmcode_function_t *) code->start)->lambda;
11291137

11301138
function = njs_function_alloc(vm, lambda, (njs_bool_t) async);
11311139
if (njs_slow_path(function == NULL)) {

‎src/test/njs_unit_test.c‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14189,22 +14189,22 @@ static njs_unit_test_t njs_test[] =
1418914189
njs_str("true") },
1419014190

1419114191
{ njs_str("new Function('('.repeat(2**13));"),
14192-
njs_str("SyntaxError: Unexpected token \"}\" in runtime:1") },
14192+
njs_str("SyntaxError: Unexpected token \"}\" in runtime") },
1419314193

1419414194
{ njs_str("new Function('{'.repeat(2**13));"),
14195-
njs_str("SyntaxError: Unexpected token \")\" in runtime:1") },
14195+
njs_str("SyntaxError: Unexpected token \")\" in runtime") },
1419614196

1419714197
{ njs_str("new Function('['.repeat(2**13));"),
14198-
njs_str("SyntaxError: Unexpected token \"}\" in runtime:1") },
14198+
njs_str("SyntaxError: Unexpected token \"}\" in runtime") },
1419914199

1420014200
{ njs_str("new Function('`'.repeat(2**13));"),
1420114201
njs_str("[object Function]") },
1420214202

1420314203
{ njs_str("new Function('{['.repeat(2**13));"),
14204-
njs_str("SyntaxError: Unexpected token \"}\" in runtime:1") },
14204+
njs_str("SyntaxError: Unexpected token \"}\" in runtime") },
1420514205

1420614206
{ njs_str("new Function('{;'.repeat(2**13));"),
14207-
njs_str("SyntaxError: Unexpected token \")\" in runtime:1") },
14207+
njs_str("SyntaxError: Unexpected token \")\" in runtime") },
1420814208

1420914209
{ njs_str("(new Function('1;'.repeat(2**13) + 'return 2'))()"),
1421014210
njs_str("2") },
@@ -14216,7 +14216,7 @@ static njs_unit_test_t njs_test[] =
1421614216
njs_str("-4") },
1421714217

1421814218
{ njs_str("new Function('new '.repeat(2**13));"),
14219-
njs_str("SyntaxError: Unexpected token \"}\" in runtime:1") },
14219+
njs_str("SyntaxError: Unexpected token \"}\" in runtime") },
1422014220

1422114221
{ njs_str("(new Function('return ' + 'typeof '.repeat(2**13) + 'x'))()"),
1422214222
njs_str("string") },
@@ -14282,7 +14282,13 @@ static njs_unit_test_t njs_test[] =
1428214282
njs_str("ReferenceError: \"foo\" is not defined") },
1428314283

1428414284
{ njs_str("this.NN = {}; var f = Function('eval = 42;'); f()"),
14285-
njs_str("SyntaxError: Identifier \"eval\" is forbidden as left-hand in assignment in runtime:1") },
14285+
njs_str("SyntaxError: Identifier \"eval\" is forbidden as left-hand in assignment in runtime") },
14286+
14287+
{ njs_str("new Function('}); let a; a; function o(){}; //')"),
14288+
njs_str("SyntaxError: Unexpected token \"}\" in runtime") },
14289+
14290+
{ njs_str("new Function('}); let a; a; function o(){}; ({')"),
14291+
njs_str("SyntaxError: single function literal required") },
1428614292

1428714293
{ njs_str("RegExp()"),
1428814294
njs_str("/(?:)/") },
@@ -19811,7 +19817,7 @@ static njs_unit_test_t njs_test[] =
1981119817
njs_str("[object AsyncFunction]") },
1981219818

1981319819
{ njs_str("let f = new Function('x', 'await 1; return x'); f(1)"),
19814-
njs_str("SyntaxError: await is only valid in async functions in runtime:1") },
19820+
njs_str("SyntaxError: await is only valid in async functions in runtime") },
1981519821

1981619822
{ njs_str("new AsyncFunction()"),
1981719823
njs_str("ReferenceError: \"AsyncFunction\" is not defined") },
@@ -21676,7 +21682,9 @@ njs_process_test(njs_external_state_t *state, njs_opts_t *opts,
2167621682
return NJS_ERROR;
2167721683
}
2167821684

21679-
success = njs_strstr_eq(&expected->ret, &s);
21685+
success = expected->ret.length <= s.length
21686+
&& (memcmp(expected->ret.start, s.start, expected->ret.length)
21687+
== 0);
2168021688
if (!success) {
2168121689
njs_stderror("njs(\"%V\")\nexpected: \"%V\"\n got: \"%V\"\n",
2168221690
&expected->script, &expected->ret, &s);

0 commit comments

Comments
 (0)