Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Dangerous abort possible in HandleOKCallback #69

Closed
springmeyer opened this issue Mar 10, 2018 · 1 comment · Fixed by #81
Closed

Dangerous abort possible in HandleOKCallback #69

springmeyer opened this issue Mar 10, 2018 · 1 comment · Fixed by #81
Milestone
v0.2.0

Comments

@springmeyer
Copy link
Contributor

springmeyer commented Mar 10, 2018 •
edited
Loading

It is possible for the code inside

vtquery/src/vtquery.cpp

Lines 362 to 418 in c3bad9b

void HandleOKCallback() override {
Nan::HandleScope scope;
v8::Local<v8::Object> results_object = Nan::New<v8::Object>();
v8::Local<v8::Array> features_array = Nan::New<v8::Array>();
results_object->Set(Nan::New("type").ToLocalChecked(), Nan::New<v8::String>("FeatureCollection").ToLocalChecked());
// for each result object
while (!results_queue_.empty()) {
auto const& feature = results_queue_.back(); // get reference to top item in results queue
if (feature.distance < std::numeric_limits<double>::max()) {
// if this is a default value, don't use it
v8::Local<v8::Object> feature_obj = Nan::New<v8::Object>();
feature_obj->Set(Nan::New("type").ToLocalChecked(), Nan::New<v8::String>("Feature").ToLocalChecked());
// create geometry object
v8::Local<v8::Object> geometry_obj = Nan::New<v8::Object>();
geometry_obj->Set(Nan::New("type").ToLocalChecked(), Nan::New<v8::String>("Point").ToLocalChecked());
v8::Local<v8::Array> coordinates_array = Nan::New<v8::Array>(2);
coordinates_array->Set(0, Nan::New<v8::Number>(feature.coordinates.x)); // latitude
coordinates_array->Set(1, Nan::New<v8::Number>(feature.coordinates.y)); // longitude
geometry_obj->Set(Nan::New("coordinates").ToLocalChecked(), coordinates_array);
feature_obj->Set(Nan::New("geometry").ToLocalChecked(), geometry_obj);
// create properties object
v8::Local<v8::Object> properties_obj = Nan::New<v8::Object>();
for (auto const& prop : feature.properties_vector) {
set_property(prop, properties_obj);
}
// set properties.tilquery
v8::Local<v8::Object> tilequery_properties_obj = Nan::New<v8::Object>();
tilequery_properties_obj->Set(Nan::New("distance").ToLocalChecked(), Nan::New<v8::Number>(feature.distance));
std::string og_geom = getGeomTypeString(feature.original_geometry_type);
tilequery_properties_obj->Set(Nan::New("geometry").ToLocalChecked(), Nan::New<v8::String>(og_geom).ToLocalChecked());
tilequery_properties_obj->Set(Nan::New("layer").ToLocalChecked(), Nan::New<v8::String>(feature.layer_name).ToLocalChecked());
properties_obj->Set(Nan::New("tilequery").ToLocalChecked(), tilequery_properties_obj);
// add properties to feature
feature_obj->Set(Nan::New("properties").ToLocalChecked(), properties_obj);
// add feature to features array
features_array->Set(static_cast<uint32_t>(results_queue_.size() - 1), feature_obj);
}
results_queue_.pop_back();
}
results_object->Set(Nan::New("features").ToLocalChecked(), features_array);
auto const argc = 2u;
v8::Local<v8::Value> argv[argc] = {
Nan::Null(), results_object};
callback->Call(argc, static_cast<v8::Local<v8::Value>*>(argv));
}
};
to suffer from a C++ exception. Because there is no try/catch this exception will lead to an abort which would bring down the entire node process. So, this needs a try/catch to protect from DDOS potential on the API side.

The particular case where I noticed this was this backtrace:

Core file '/cores/core.46375' (x86_64) was loaded.
(lldb) thread backtrace all
* thread #1, stop reason = signal SIGSTOP
  * frame #0: 0x00007fff50f6ae3e libsystem_kernel.dylib`__pthread_kill + 10
    frame #1: 0x00007fff510a9150 libsystem_pthread.dylib`pthread_kill + 333
    frame #2: 0x00007fff50ec7312 libsystem_c.dylib`abort + 127
    frame #3: 0x00007fff4eea2f8f libc++abi.dylib`abort_message + 245
    frame #4: 0x00007fff4eea3113 libc++abi.dylib`default_terminate_handler() + 241
    frame #5: 0x00007fff5022deab libobjc.A.dylib`_objc_terminate() + 105
    frame #6: 0x00007fff4eebe7c9 libc++abi.dylib`std::__terminate(void (*)()) + 8
    frame #7: 0x00007fff4eebe26d libc++abi.dylib`__cxa_throw + 121
    frame #8: 0x0000000102bf479c vtquery.node`protozero::pbf_reader::next() + 204
    frame #9: 0x0000000102c0046b vtquery.node`vtzero::property_value::type() const + 43
    frame #10: 0x0000000102c002d5 vtquery.node`_ZN6vtzero13apply_visitorINS_6detail15convert_visitorIN6mapbox7feature5valueENS3_11vector_tile6detail22property_value_mappingEEEEEDTclclsr3stdE7declvalIT_EEtlN9protozero9data_viewEEEEOSA_NS_14property_valueE + 37
    frame #11: 0x0000000102bf0cfd vtquery.node`VectorTileQuery::set_property(vtzero::property const&, v8::Local<v8::Object>&) + 61
    frame #12: 0x0000000102bf609b vtquery.node`VectorTileQuery::Worker::HandleOKCallback() + 731
    frame #13: 0x0000000102bf4f4a vtquery.node`Nan::AsyncWorker::WorkComplete() + 58
    frame #14: 0x0000000102c00292 vtquery.node`Nan::AsyncExecuteComplete(uv_work_s*) + 18
    frame #15: 0x0000000100a5ff65 node`uv__work_done + 178
    frame #16: 0x0000000100a61e0e node`uv__async_io + 317
    frame #17: 0x0000000100a71e74 node`uv__io_poll + 1934
    frame #18: 0x0000000100a6228f node`uv_run + 321
    frame #19: 0x00000001008ce97d node`node::Start(int, char**) + 744
    frame #20: 0x0000000100001234 node`start + 52
  thread #2, stop reason = signal SIGSTOP
    frame #0: 0x00007fff50f617fe libsystem_kernel.dylib`semaphore_wait_trap + 10
    frame #1: 0x0000000100a6d378 node`uv_sem_wait + 16
    frame #2: 0x00000001008cf6b4 node`node::DebugSignalThreadMain(void*) + 49
    frame #3: 0x00007fff510a66c1 libsystem_pthread.dylib`_pthread_body + 340
    frame #4: 0x00007fff510a656d libsystem_pthread.dylib`_pthread_start + 377
    frame #5: 0x00007fff510a5c5d libsystem_pthread.dylib`thread_start + 13
  thread #3, stop reason = signal SIGSTOP
    frame #0: 0x00007fff50f617fe libsystem_kernel.dylib`semaphore_wait_trap + 10
    frame #1: 0x0000000100a75647 node`v8::base::Semaphore::Wait() + 23
    frame #2: 0x00000001009346a9 node`v8::platform::TaskQueue::GetNext() + 57
    frame #3: 0x00000001009348ab node`v8::platform::WorkerThread::Run() + 43
    frame #4: 0x0000000100a77357 node`v8::base::ThreadEntry(void*) + 87
    frame #5: 0x00007fff510a66c1 libsystem_pthread.dylib`_pthread_body + 340
    frame #6: 0x00007fff510a656d libsystem_pthread.dylib`_pthread_start + 377
    frame #7: 0x00007fff510a5c5d libsystem_pthread.dylib`thread_start + 13
  thread #4, stop reason = signal SIGSTOP
    frame #0: 0x00007fff50f617fe libsystem_kernel.dylib`semaphore_wait_trap + 10
    frame #1: 0x0000000100a75647 node`v8::base::Semaphore::Wait() + 23
    frame #2: 0x00000001009346a9 node`v8::platform::TaskQueue::GetNext() + 57
    frame #3: 0x00000001009348ab node`v8::platform::WorkerThread::Run() + 43
    frame #4: 0x0000000100a77357 node`v8::base::ThreadEntry(void*) + 87
    frame #5: 0x00007fff510a66c1 libsystem_pthread.dylib`_pthread_body + 340
    frame #6: 0x00007fff510a656d libsystem_pthread.dylib`_pthread_start + 377
    frame #7: 0x00007fff510a5c5d libsystem_pthread.dylib`thread_start + 13
  thread #5, stop reason = signal SIGSTOP
    frame #0: 0x00007fff50f617fe libsystem_kernel.dylib`semaphore_wait_trap + 10
    frame #1: 0x0000000100a75647 node`v8::base::Semaphore::Wait() + 23
    frame #2: 0x00000001009346a9 node`v8::platform::TaskQueue::GetNext() + 57
    frame #3: 0x00000001009348ab node`v8::platform::WorkerThread::Run() + 43
    frame #4: 0x0000000100a77357 node`v8::base::ThreadEntry(void*) + 87
    frame #5: 0x00007fff510a66c1 libsystem_pthread.dylib`_pthread_body + 340
    frame #6: 0x00007fff510a656d libsystem_pthread.dylib`_pthread_start + 377
    frame #7: 0x00007fff510a5c5d libsystem_pthread.dylib`thread_start + 13
  thread #6, stop reason = signal SIGSTOP
    frame #0: 0x00007fff50f617fe libsystem_kernel.dylib`semaphore_wait_trap + 10
    frame #1: 0x0000000100a75647 node`v8::base::Semaphore::Wait() + 23
    frame #2: 0x00000001009346a9 node`v8::platform::TaskQueue::GetNext() + 57
    frame #3: 0x00000001009348ab node`v8::platform::WorkerThread::Run() + 43
    frame #4: 0x0000000100a77357 node`v8::base::ThreadEntry(void*) + 87
    frame #5: 0x00007fff510a66c1 libsystem_pthread.dylib`_pthread_body + 340
    frame #6: 0x00007fff510a656d libsystem_pthread.dylib`_pthread_start + 377
    frame #7: 0x00007fff510a5c5d libsystem_pthread.dylib`thread_start + 13
  thread #7, stop reason = signal SIGSTOP
    frame #0: 0x00007fff50f6acee libsystem_kernel.dylib`__psynch_cvwait + 10
    frame #1: 0x00007fff510a7662 libsystem_pthread.dylib`_pthread_cond_wait + 732
    frame #2: 0x0000000100a6d49a node`uv_cond_wait + 9
    frame #3: 0x0000000100a602c9 node`worker + 227
    frame #4: 0x00007fff510a66c1 libsystem_pthread.dylib`_pthread_body + 340
    frame #5: 0x00007fff510a656d libsystem_pthread.dylib`_pthread_start + 377
    frame #6: 0x00007fff510a5c5d libsystem_pthread.dylib`thread_start + 13
  thread #8, stop reason = signal SIGSTOP
    frame #0: 0x00007fff50f6b432 libsystem_kernel.dylib`__ulock_wait + 10
    frame #1: 0x00007fff5109f6ba libsystem_platform.dylib`_os_unfair_lock_lock_slow + 140
    frame #2: 0x00007fff50fc3332 libsystem_malloc.dylib`szone_malloc_should_clear + 213
    frame #3: 0x00007fff50fc3201 libsystem_malloc.dylib`malloc_zone_malloc + 103
    frame #4: 0x00007fff50fc250b libsystem_malloc.dylib`malloc + 24
    frame #5: 0x00007fff4eea2628 libc++abi.dylib`operator new(unsigned long) + 40
    frame #6: 0x0000000102bfb383 vtquery.node`std::__1::vector<mapbox::geometry::point<long long>, std::__1::allocator<mapbox::geometry::point<long long> > >::reserve(unsigned long) + 67
    frame #7: 0x0000000102bfbcfc vtquery.node`vtzero::detail::get_result<mapbox::vector_tile::detail::polygon_geometry_handler<long long>, void>::type vtzero::detail::geometry_decoder<protozero::const_varint_iterator<unsigned int> >::decode_polygon<mapbox::vector_tile::detail::polygon_geometry_handler<long long> >(mapbox::vector_tile::detail::polygon_geometry_handler<long long>&&) + 364
    frame #8: 0x0000000102bfa6c9 vtquery.node`mapbox::geometry::geometry<long long, std::__1::vector> mapbox::vector_tile::detail::extract_geometry_polygon<long long>(vtzero::feature const&) + 105
    frame #9: 0x0000000102bf5779 vtquery.node`VectorTileQuery::Worker::Execute() + 1993
    frame #10: 0x0000000100a60240 node`worker + 90
    frame #11: 0x00007fff510a66c1 libsystem_pthread.dylib`_pthread_body + 340
    frame #12: 0x00007fff510a656d libsystem_pthread.dylib`_pthread_start + 377
    frame #13: 0x00007fff510a5c5d libsystem_pthread.dylib`thread_start + 13
  thread #9, stop reason = signal SIGSTOP
    frame #0: 0x00007fff50fa1221 libsystem_m.dylib`___lldb_unnamed_symbol122$$libsystem_m.dylib + 72
    frame #1: 0x0000000102bf58f1 vtquery.node`VectorTileQuery::Worker::Execute() + 2369
    frame #2: 0x0000000100a60240 node`worker + 90
    frame #3: 0x00007fff510a66c1 libsystem_pthread.dylib`_pthread_body + 340
    frame #4: 0x00007fff510a656d libsystem_pthread.dylib`_pthread_start + 377
    frame #5: 0x00007fff510a5c5d libsystem_pthread.dylib`thread_start + 13
  thread #10, stop reason = signal SIGSTOP
    frame #0: 0x00007fff50f6acee libsystem_kernel.dylib`__psynch_cvwait + 10
    frame #1: 0x00007fff510a7662 libsystem_pthread.dylib`_pthread_cond_wait + 732
    frame #2: 0x0000000100a6d49a node`uv_cond_wait + 9
    frame #3: 0x0000000100a602c9 node`worker + 227
    frame #4: 0x00007fff510a66c1 libsystem_pthread.dylib`_pthread_body + 340
    frame #5: 0x00007fff510a656d libsystem_pthread.dylib`_pthread_start + 377
    frame #6: 0x00007fff510a5c5d libsystem_pthread.dylib`thread_start + 13
  thread #11, stop reason = signal SIGSTOP
    frame #0: 0x0000000102bfbc4a vtquery.node`vtzero::detail::get_result<mapbox::vector_tile::detail::polygon_geometry_handler<long long>, void>::type vtzero::detail::geometry_decoder<protozero::const_varint_iterator<unsigned int> >::decode_polygon<mapbox::vector_tile::detail::polygon_geometry_handler<long long> >(mapbox::vector_tile::detail::polygon_geometry_handler<long long>&&) + 186
    frame #1: 0x0000000102bfa6c9 vtquery.node`mapbox::geometry::geometry<long long, std::__1::vector> mapbox::vector_tile::detail::extract_geometry_polygon<long long>(vtzero::feature const&) + 105
    frame #2: 0x0000000102bf5779 vtquery.node`VectorTileQuery::Worker::Execute() + 1993
    frame #3: 0x0000000100a60240 node`worker + 90
    frame #4: 0x00007fff510a66c1 libsystem_pthread.dylib`_pthread_body + 340
    frame #5: 0x00007fff510a656d libsystem_pthread.dylib`_pthread_start + 377
    frame #6: 0x00007fff510a5c5d libsystem_pthread.dylib`thread_start + 13
  thread #12, stop reason = signal SIGSTOP
    frame #0: 0x00007fff50c0c8af libz.1.dylib`___lldb_unnamed_symbol42$$libz.1.dylib + 175
    frame #1: 0x00007fff50c096b7 libz.1.dylib`inflate + 5432
    frame #2: 0x0000000102bf6e28 vtquery.node`void gzip::Decompressor::decompress<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >&, char const*, unsigned long) const + 232
    frame #3: 0x0000000102bf5180 vtquery.node`VectorTileQuery::Worker::Execute() + 464
    frame #4: 0x0000000100a60240 node`worker + 90
    frame #5: 0x00007fff510a66c1 libsystem_pthread.dylib`_pthread_body + 340
    frame #6: 0x00007fff510a656d libsystem_pthread.dylib`_pthread_start + 377
    frame #7: 0x00007fff510a5c5d libsystem_pthread.dylib`thread_start + 13
  thread #13, stop reason = signal SIGSTOP
    frame #0: 0x00007fff50fc4f44 libsystem_malloc.dylib`tiny_free_list_add_ptr + 227
    frame #1: 0x00007fff50fda758 libsystem_malloc.dylib`tiny_free_no_lock + 570
    frame #2: 0x00007fff50fdb254 libsystem_malloc.dylib`free_tiny + 628
    frame #3: 0x0000000102bf582d vtquery.node`VectorTileQuery::Worker::Execute() + 2173
    frame #4: 0x0000000100a60240 node`worker + 90
    frame #5: 0x00007fff510a66c1 libsystem_pthread.dylib`_pthread_body + 340
    frame #6: 0x00007fff510a656d libsystem_pthread.dylib`_pthread_start + 377
    frame #7: 0x00007fff510a5c5d libsystem_pthread.dylib`thread_start + 13
  thread #14, stop reason = signal SIGSTOP
    frame #0: 0x0000000102bfba76 vtquery.node`std::__1::vector<mapbox::geometry::polygon<long long, std::__1::vector>, std::__1::allocator<mapbox::geometry::polygon<long long, std::__1::vector> > >::reserve(unsigned long) + 214
    frame #1: 0x0000000102bfa6f9 vtquery.node`mapbox::geometry::geometry<long long, std::__1::vector> mapbox::vector_tile::detail::extract_geometry_polygon<long long>(vtzero::feature const&) + 153
    frame #2: 0x0000000102bf5779 vtquery.node`VectorTileQuery::Worker::Execute() + 1993
    frame #3: 0x0000000100a60240 node`worker + 90
    frame #4: 0x00007fff510a66c1 libsystem_pthread.dylib`_pthread_body + 340
    frame #5: 0x00007fff510a656d libsystem_pthread.dylib`_pthread_start + 377
    frame #6: 0x00007fff510a5c5d libsystem_pthread.dylib`thread_start + 13
  thread #15, stop reason = signal SIGSTOP
    frame #0: 0x00007fff50f6acee libsystem_kernel.dylib`__psynch_cvwait + 10
    frame #1: 0x00007fff510a7662 libsystem_pthread.dylib`_pthread_cond_wait + 732
    frame #2: 0x0000000100a6d49a node`uv_cond_wait + 9
    frame #3: 0x0000000100a602c9 node`worker + 227
    frame #4: 0x00007fff510a66c1 libsystem_pthread.dylib`_pthread_body + 340
    frame #5: 0x00007fff510a656d libsystem_pthread.dylib`_pthread_start + 377
    frame #6: 0x00007fff510a5c5d libsystem_pthread.dylib`thread_start + 13
  thread #16, stop reason = signal SIGSTOP
    frame #0: 0x00007fff50fc56a1 libsystem_malloc.dylib`tiny_free_list_remove_ptr + 96
    frame #1: 0x00007fff50fdaac8 libsystem_malloc.dylib`tiny_free_no_lock + 1450
    frame #2: 0x00007fff50fdb254 libsystem_malloc.dylib`free_tiny + 628
    frame #3: 0x0000000102bfc584 vtquery.node`mapbox::util::detail::variant_helper<mapbox::geometry::polygon<long long, std::__1::vector>, mapbox::geometry::multi_point<long long, std::__1::vector>, mapbox::geometry::multi_line_string<long long, std::__1::vector>, mapbox::geometry::multi_polygon<long long, std::__1::vector>, mapbox::geometry::geometry_collection<long long, std::__1::vector> >::destroy(unsigned long, void*) + 100
    frame #4: 0x0000000102bf582d vtquery.node`VectorTileQuery::Worker::Execute() + 2173
    frame #5: 0x0000000100a60240 node`worker + 90
    frame #6: 0x00007fff510a66c1 libsystem_pthread.dylib`_pthread_body + 340
    frame #7: 0x00007fff510a656d libsystem_pthread.dylib`_pthread_start + 377
    frame #8: 0x00007fff510a5c5d libsystem_pthread.dylib`thread_start + 13
(lldb) quit
This was referenced Apr 4, 2018
Open
Closed
@mapsam mapsam mentioned this issue Apr 13, 2018
Merged
@mapsam mapsam added this to the v0.2.0 milestone Apr 13, 2018
@mapsam
Copy link
Contributor

mapsam commented Apr 13, 2018

Hey @springmeyer I've added the try/catch logic in #81, but also added some exclude comment for code coverage since I'm not exactly sure to recreate in unit tests. Any ideas?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.2.0
Development

Successfully merging a pull request may close this issue.

add try/catch to HandleOKCallback mapbox/vtquery
2 participants
@springmeyer @mapsam