Skip to content

Commit 75829d8

Browse files
daeyeonyichoi
authored andcommitted
Add a more secure type validator to Buffer (#911)
This commit add a validator to check whether the execution context on this is not the expected C type. #816 IoT.js-DCO-1.0-Signed-off-by: Daeyeon Jeong [email protected]
1 parent 7a7e845 commit 75829d8

File tree

6 files changed

+84
-40
lines changed

6 files changed

+84
-40
lines changed

src/iotjs_binding.c

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,31 @@ uintptr_t iotjs_jval_get_object_native_handle(const iotjs_jval_t* jobj) {
384384
}
385385

386386

387+
uintptr_t iotjs_jval_get_object_from_jhandler(iotjs_jhandler_t* jhandler,
388+
JNativeInfoType native_info) {
389+
const iotjs_jval_t* jobj = JHANDLER_GET_THIS(object);
390+
const IOTJS_DECLARE_THIS(iotjs_jval_t, jobj);
391+
392+
if (!jerry_value_is_object(_this->value)) {
393+
return 0;
394+
}
395+
396+
uintptr_t ptr = 0;
397+
JNativeInfoType out_native_info;
398+
399+
if (jerry_get_object_native_pointer(_this->value, (void**)&ptr,
400+
&out_native_info)) {
401+
if (ptr && out_native_info == native_info) {
402+
return ptr;
403+
}
404+
}
405+
406+
JHANDLER_THROW(COMMON, "Unsafe access");
407+
408+
return 0;
409+
}
410+
411+
387412
void iotjs_jval_set_property_by_index(const iotjs_jval_t* jarr, uint32_t idx,
388413
const iotjs_jval_t* jval) {
389414
const IOTJS_VALIDATED_STRUCT_METHOD(iotjs_jval_t, jarr);

src/iotjs_binding.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,8 @@ iotjs_jval_t iotjs_jval_get_property(THIS_JVAL, const char* name);
128128
void iotjs_jval_set_object_native_handle(THIS_JVAL, uintptr_t ptr,
129129
JNativeInfoType native_info);
130130
uintptr_t iotjs_jval_get_object_native_handle(THIS_JVAL);
131+
uintptr_t iotjs_jval_get_object_from_jhandler(iotjs_jhandler_t* jhandler,
132+
JNativeInfoType native_info);
131133

132134
void iotjs_jval_set_property_by_index(THIS_JVAL, uint32_t idx,
133135
const iotjs_jval_t* value);
@@ -307,6 +309,13 @@ static inline bool ge(uint16_t a, uint16_t b) {
307309
JHANDLER_CHECK_ARG_IF_EXIST(index, type)
308310
#endif
309311

312+
#define JHANDLER_DECLARE_THIS_PTR(type, name) \
313+
iotjs_##type##_t* name = (iotjs_##type##_t*) \
314+
iotjs_jval_get_object_from_jhandler(jhandler, &this_module_native_info); \
315+
if (!name) { \
316+
return; \
317+
}
318+
310319
void iotjs_binding_initialize();
311320
void iotjs_binding_finalize();
312321

src/iotjs_objectwrap.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,4 +41,10 @@ iotjs_jobjectwrap_t* iotjs_jobjectwrap_from_jobject(
4141
.free_cb = (jerry_object_native_free_callback_t)iotjs_##module##_destroy \
4242
}
4343

44+
#define IOTJS_DEFINE_NATIVE_HANDLE_INFO_THIS_MODULE(name) \
45+
static void iotjs_##name##_destroy(iotjs_##name##_t* wrap); \
46+
static const jerry_object_native_info_t this_module_native_info = { \
47+
.free_cb = (jerry_object_native_free_callback_t)iotjs_##name##_destroy \
48+
}
49+
4450
#endif /* IOTJS_OBJECTWRAP_H */

src/modules/iotjs_module_buffer.c

Lines changed: 11 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,7 @@
2121
#include <string.h>
2222

2323

24-
static void iotjs_bufferwrap_destroy(iotjs_bufferwrap_t* bufferwrap);
25-
IOTJS_DEFINE_NATIVE_HANDLE_INFO(bufferwrap);
24+
IOTJS_DEFINE_NATIVE_HANDLE_INFO_THIS_MODULE(bufferwrap);
2625

2726

2827
iotjs_bufferwrap_t* iotjs_bufferwrap_create(const iotjs_jval_t* jbuiltin,
@@ -31,7 +30,7 @@ iotjs_bufferwrap_t* iotjs_bufferwrap_create(const iotjs_jval_t* jbuiltin,
3130
IOTJS_VALIDATED_STRUCT_CONSTRUCTOR(iotjs_bufferwrap_t, bufferwrap);
3231

3332
iotjs_jobjectwrap_initialize(&_this->jobjectwrap, jbuiltin,
34-
&bufferwrap_native_info);
33+
&this_module_native_info);
3534
if (length > 0) {
3635
_this->length = length;
3736
_this->buffer = iotjs_buffer_allocate(length);
@@ -237,13 +236,9 @@ JHANDLER_FUNCTION(Buffer) {
237236

238237

239238
JHANDLER_FUNCTION(Compare) {
240-
DJHANDLER_CHECK_THIS(object);
239+
JHANDLER_DECLARE_THIS_PTR(bufferwrap, src_buffer_wrap);
241240
DJHANDLER_CHECK_ARGS(1, object);
242241

243-
const iotjs_jval_t* jsrc_builtin = JHANDLER_GET_THIS(object);
244-
iotjs_bufferwrap_t* src_buffer_wrap =
245-
iotjs_bufferwrap_from_jbuiltin(jsrc_builtin);
246-
247242
const iotjs_jval_t* jdst_buffer = JHANDLER_GET_ARG(0, object);
248243
iotjs_bufferwrap_t* dst_buffer_wrap =
249244
iotjs_bufferwrap_from_jbuffer(jdst_buffer);
@@ -265,13 +260,9 @@ JHANDLER_FUNCTION(Compare) {
265260

266261

267262
JHANDLER_FUNCTION(Copy) {
268-
DJHANDLER_CHECK_THIS(object);
263+
JHANDLER_DECLARE_THIS_PTR(bufferwrap, src_buffer_wrap);
269264
DJHANDLER_CHECK_ARGS(4, object, number, number, number);
270265

271-
const iotjs_jval_t* jsrc_builtin = JHANDLER_GET_THIS(object);
272-
iotjs_bufferwrap_t* src_buffer_wrap =
273-
iotjs_bufferwrap_from_jbuiltin(jsrc_builtin);
274-
275266
const iotjs_jval_t* jdst_buffer = JHANDLER_GET_ARG(0, object);
276267
iotjs_bufferwrap_t* dst_buffer_wrap =
277268
iotjs_bufferwrap_from_jbuffer(jdst_buffer);
@@ -301,14 +292,11 @@ JHANDLER_FUNCTION(Copy) {
301292

302293

303294
JHANDLER_FUNCTION(Write) {
295+
JHANDLER_DECLARE_THIS_PTR(bufferwrap, buffer_wrap);
304296
DJHANDLER_CHECK_ARGS(3, string, number, number);
305297

306298
iotjs_string_t src = JHANDLER_GET_ARG(0, string);
307299

308-
const iotjs_jval_t* jbuiltin = JHANDLER_GET_THIS(object);
309-
310-
iotjs_bufferwrap_t* buffer_wrap = iotjs_bufferwrap_from_jbuiltin(jbuiltin);
311-
312300
size_t buffer_length = iotjs_bufferwrap_length(buffer_wrap);
313301
DECLARE_SIZE_T_FROM_DOUBLE(offset, JHANDLER_GET_ARG(1, number));
314302
offset = bound_range(offset, 0, buffer_length);
@@ -328,15 +316,12 @@ JHANDLER_FUNCTION(Write) {
328316

329317

330318
JHANDLER_FUNCTION(WriteUInt8) {
319+
JHANDLER_DECLARE_THIS_PTR(bufferwrap, buffer_wrap);
331320
DJHANDLER_CHECK_ARGS(2, number, number);
332321

333322
const char src[] = { (char)JHANDLER_GET_ARG(0, number) };
334323
size_t length = 1;
335324

336-
const iotjs_jval_t* jbuiltin = JHANDLER_GET_THIS(object);
337-
338-
iotjs_bufferwrap_t* buffer_wrap = iotjs_bufferwrap_from_jbuiltin(jbuiltin);
339-
340325
size_t buffer_length = iotjs_bufferwrap_length(buffer_wrap);
341326
DECLARE_SIZE_T_FROM_DOUBLE(offset, JHANDLER_GET_ARG(1, number));
342327
offset = bound_range(offset, 0, buffer_length);
@@ -351,14 +336,11 @@ JHANDLER_FUNCTION(WriteUInt8) {
351336

352337

353338
JHANDLER_FUNCTION(HexWrite) {
339+
JHANDLER_DECLARE_THIS_PTR(bufferwrap, buffer_wrap);
354340
DJHANDLER_CHECK_ARGS(3, string, number, number);
355341

356342
iotjs_string_t src = JHANDLER_GET_ARG(0, string);
357343

358-
const iotjs_jval_t* jbuiltin = JHANDLER_GET_THIS(object);
359-
360-
iotjs_bufferwrap_t* buffer_wrap = iotjs_bufferwrap_from_jbuiltin(jbuiltin);
361-
362344
size_t buffer_length = iotjs_bufferwrap_length(buffer_wrap);
363345
DECLARE_SIZE_T_FROM_DOUBLE(offset, JHANDLER_GET_ARG(1, number));
364346
offset = bound_range(offset, 0, buffer_length);
@@ -382,12 +364,9 @@ JHANDLER_FUNCTION(HexWrite) {
382364

383365

384366
JHANDLER_FUNCTION(ReadUInt8) {
367+
JHANDLER_DECLARE_THIS_PTR(bufferwrap, buffer_wrap);
385368
DJHANDLER_CHECK_ARGS(1, number);
386369

387-
const iotjs_jval_t* jbuiltin = JHANDLER_GET_THIS(object);
388-
389-
iotjs_bufferwrap_t* buffer_wrap = iotjs_bufferwrap_from_jbuiltin(jbuiltin);
390-
391370
size_t buffer_length = iotjs_bufferwrap_length(buffer_wrap);
392371
DECLARE_SIZE_T_FROM_DOUBLE(offset, JHANDLER_GET_ARG(0, number));
393372
offset = bound_range(offset, 0, buffer_length - 1);
@@ -399,11 +378,9 @@ JHANDLER_FUNCTION(ReadUInt8) {
399378

400379

401380
JHANDLER_FUNCTION(Slice) {
381+
JHANDLER_DECLARE_THIS_PTR(bufferwrap, buffer_wrap);
402382
DJHANDLER_CHECK_ARGS(2, number, number);
403383

404-
const iotjs_jval_t* jbuiltin = JHANDLER_GET_THIS(object);
405-
iotjs_bufferwrap_t* buffer_wrap = iotjs_bufferwrap_from_jbuiltin(jbuiltin);
406-
407384
int64_t start = JHANDLER_GET_ARG(0, number);
408385
int64_t end = JHANDLER_GET_ARG(1, number);
409386
size_t start_idx, end_idx;
@@ -451,12 +428,9 @@ JHANDLER_FUNCTION(Slice) {
451428

452429

453430
JHANDLER_FUNCTION(ToString) {
454-
DJHANDLER_CHECK_THIS(object);
431+
JHANDLER_DECLARE_THIS_PTR(bufferwrap, buffer_wrap);
455432
DJHANDLER_CHECK_ARGS(2, number, number);
456433

457-
const iotjs_jval_t* jbuiltin = JHANDLER_GET_THIS(object);
458-
iotjs_bufferwrap_t* buffer_wrap = iotjs_bufferwrap_from_jbuiltin(jbuiltin);
459-
460434
DECLARE_SIZE_T_FROM_DOUBLE(start, JHANDLER_GET_ARG(0, number));
461435
start = bound_range(start, 0, iotjs_bufferwrap_length(buffer_wrap));
462436
DECLARE_SIZE_T_FROM_DOUBLE(end, JHANDLER_GET_ARG(1, number));
@@ -479,11 +453,9 @@ JHANDLER_FUNCTION(ToString) {
479453

480454

481455
JHANDLER_FUNCTION(ToHexString) {
456+
JHANDLER_DECLARE_THIS_PTR(bufferwrap, buffer_wrap);
482457
DJHANDLER_CHECK_THIS(object);
483458

484-
const iotjs_jval_t* jbuiltin = JHANDLER_GET_THIS(object);
485-
iotjs_bufferwrap_t* buffer_wrap = iotjs_bufferwrap_from_jbuiltin(jbuiltin);
486-
487459
size_t length = iotjs_bufferwrap_length(buffer_wrap);
488460
const char* data = iotjs_bufferwrap_buffer(buffer_wrap);
489461

test/run_pass/issue/issue-816.js

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/* Copyright 2017-present Samsung Electronics Co., Ltd. and other contributors
2+
*
3+
* Licensed under the Apache License, Version 2.0 (the "License");
4+
* you may not use this file except in compliance with the License.
5+
* You may obtain a copy of the License at
6+
*
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
*
9+
* Unless required by applicable law or agreed to in writing, software
10+
* distributed under the License is distributed on an "AS IS" BASIS
11+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
* See the License for the specific language governing permissions and
13+
* limitations under the License.
14+
*/
15+
16+
var Buffer = require('buffer');
17+
var assert = require('assert');
18+
19+
assert.throws(function () {
20+
var buf = new Buffer("ABCDEF");
21+
var o = {
22+
_builtin: {
23+
compare: function () {
24+
buf._builtin.compare.call(this, arguments);
25+
}
26+
},
27+
compare: buf.compare
28+
};
29+
30+
o.compare(buf);
31+
}, Error);

test/testsets.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,8 @@
8989
{ "name": "issue-198.js" },
9090
{ "name": "issue-223.js" },
9191
{ "name": "issue-266.js" },
92-
{ "name": "issue-323.js" }
92+
{ "name": "issue-323.js" },
93+
{ "name": "issue-816.js" }
9394
],
9495
"run_fail": [
9596
{ "name": "test_assert_equal.js", "expected-failure": true },

0 commit comments

Comments
 (0)