Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Crash when parsing invalid sprite URL #5723

Merged
merged 2 commits into from
Jul 19, 2016
Merged

Conversation

kkaefer
Copy link
Member

@kkaefer kkaefer commented Jul 19, 2016

When a style contains a sprite URL that is invalid, Mapbox GL Native crashes when the filename of the sprite URL does not contain a file extension.

#18. Crashed: DefaultFileSource
0  AppName                        0x1000effdc CLSProcessRecordAllThreads + 4296146908
1  AppName                        0x1000effdc CLSProcessRecordAllThreads + 4296146908
2  AppName                        0x1000f03fc CLSProcessRecordAllThreads + 4296147964
3  AppName                        0x1000e10c0 CLSHandler + 4296085696
4  AppName                        0x1000ee704 __CLSExceptionRecord_block_invoke + 4296140548
5  libdispatch.dylib              0x182fb947c _dispatch_client_callout + 16
6  libdispatch.dylib              0x182fc4728 _dispatch_barrier_sync_f_invoke + 100
7  AppName                        0x1000ee1b0 CLSExceptionRecord + 4296139184
8  AppName                        0x1000edd34 CLSTerminateHandler() + 4296138036
9  libc++abi.dylib                0x182bc6f44 std::__terminate(void (*)()) + 16
10 libc++abi.dylib                0x182bc685c __cxxabiv1::exception_cleanup_func(_Unwind_Reason_Code, _Unwind_Exception*) + 134
11 libc++abi.dylib                0x182bc7154 operator new(unsigned long) + 96
12 AppName                        0x1001b6464 void std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > >::__push_back_slow_path<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> >&&) + 60048
13 AppName                        0x1001d2554 mbgl::util::mapbox::getMapboxURLPathname(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) + 68500
14 AppName                        0x1001d2cf8 mbgl::util::mapbox::normalizeSpriteURL(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) + 70456
15 AppName                        0x1001ef61c mbgl::OnlineFileSource::request(mbgl::Resource const&, std::__1::function<void (mbgl::Response)>) + 15900
16 AppName                        0x1001ec710 mbgl::DefaultFileSource::Impl::Task::Task(mbgl::Resource, std::__1::function<void (mbgl::Response)>, mbgl::DefaultFileSource::Impl*) + 3856
17 AppName                        0x1001ebe7c mbgl::DefaultFileSource::Impl::request(mbgl::FileRequest*, mbgl::Resource, std::__1::function<void (mbgl::Response)>) + 1660
18 AppName                        0x1001e983c mbgl::util::RunLoop::Invoker<auto mbgl::util::Thread<mbgl::DefaultFileSource::Impl>::bind<void (mbgl::DefaultFileSource::Impl::*)(mbgl::FileRequest*, mbgl::Resource, std::__1::function<void (mbgl::Response)>)>(void (mbgl::DefaultFileSource::Impl::*)(mbgl::FileRequest*, mbgl::Resource, std::__1::function<void (mbgl::Response)>))::'lambda'(void (mbgl::DefaultFileSource::Impl::*&&)(mbgl::FileRequest*, mbgl::Resource, std::__1::function<void (mbgl::Response)>)), std::__1::tuple<mbgl::DefaultFileSource::request(mbgl::Resource const&, std::__1::function<void (mbgl::Response)>)::DefaultFileRequest*, mbgl::Resource, std::__1::unique_ptr<mbgl::WorkRequest, std::__1::default_delete<std::__1::default_delete> > mbgl::util::RunLoop::invokeWithCallback<std::__1::tuple, std::__1::function<void (mbgl::Response)>&, mbgl::util::RunLoop::invokeWithCallback, mbgl::Resource&>(void (mbgl::DefaultFileSource::Impl::*&&)(mbgl::FileRequest*, mbgl::Resource, std::__1::function<void (mbgl::Response)>), std::__1::function<void (mbgl::Response)>&&&, mbgl::util::RunLoop::invokeWithCallback&&, mbgl::Resource&&&)::'lambda'(auto mbgl::util::Thread<mbgl::DefaultFileSource::Impl>::bind<void (mbgl::DefaultFileSource::Impl::*)(mbgl::FileRequest*, mbgl::Resource, std::__1::function<void (mbgl::Response)>)>(void (mbgl::DefaultFileSource::Impl::*)(mbgl::FileRequest*, mbgl::Resource, std::__1::function<void (mbgl::Response)>))::'lambda'(void (mbgl::DefaultFileSource::Impl::*&&)(mbgl::FileRequest*, mbgl::Resource, std::__1::function<void (mbgl::Response)>)))> >::operator()() + 63048
19 AppName                        0x1001e7698 mbgl::util::RunLoop::process() + 54436
20 AppName                        0x100114474 uv__async_event (async.c:84)
21 AppName                        0x100114634 uv__async_io (async.c:137)
22 AppName                        0x100117070 uv__io_poll (kqueue.c:248)
23 AppName                        0x1001149d8 uv_run (core.c:342)
24 AppName                        0x1001eb9ac void mbgl::util::Thread<mbgl::DefaultFileSource::Impl>::run<std::__1::tuple<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, unsigned long long&>, 0ul, 1ul>(mbgl::util::ThreadContext, std::__1::tuple<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, unsigned long long&>&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul>) + 428
25 AppName                        0x1001eb8a8 std::__1::__thread_proxy<std::__1::tuple<mbgl::util::Thread<mbgl::DefaultFileSource::Impl>::Thread<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, unsigned long long&>(mbgl::util::ThreadContext const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&&&, unsigned long long&&&)::'lambda'()> >(void*, void*) + 168
26 libsystem_pthread.dylib        0x1831d3b28 _pthread_body + 156
27 libsystem_pthread.dylib        0x1831d3a8c _pthread_body + 154
28 libsystem_pthread.dylib        0x1831d1028 thread_start + 4

@mention-bot
Copy link

@kkaefer, thanks for your PR! By analyzing the annotation information on this pull request, we identified @lucaswoj, @tmcw and @jfirebaugh to be potential reviewers

@1ec5 1ec5 added crash Core The cross-platform C++ core, aka mbgl labels Jul 19, 2016
@1ec5 1ec5 added this to the ios-v3.3.1 milestone Jul 19, 2016
@1ec5
Copy link
Contributor

1ec5 commented Jul 19, 2016

We’d like to pull this into iOS SDK v3.3.1. That may require #5554, which is in a similar vein.

@1ec5 1ec5 mentioned this pull request Jul 19, 2016
@kkaefer
Copy link
Member Author

kkaefer commented Jul 19, 2016

Ran afl-fuzz on this and couldn't produce any crashes with this path applied.

@bleege
Copy link
Contributor

bleege commented Jul 19, 2016

FYI.. this is also going to affect Android too.

/cc @zugaldia @ivovandongen @tobrun @cammace

@kkaefer kkaefer merged commit 40ea424 into master Jul 19, 2016
@kkaefer kkaefer deleted the 5723-fix-missing-file-extension branch July 19, 2016 15:58
@@ -19,13 +19,13 @@ bool isMapboxURL(const std::string& url) {

std::vector<std::string> getMapboxURLPathname(const std::string& url) {
std::vector<std::string> pathname;
std::size_t startIndex = protocol.length();
std::size_t end = url.find_first_of("?#");
auto startIndex = protocol.length();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, I prefer to use explicit types for normal local variables, rather than seeing auto and then needing to mentally work out what the type must be.

@bleege
Copy link
Contributor

bleege commented Jul 19, 2016

Addresses #5722

@bleege bleege mentioned this pull request Jul 19, 2016
14 tasks
@bleege bleege modified the milestones: android-v4.1.1, ios-v3.3.1 Jul 19, 2016
1ec5 added a commit that referenced this pull request Jul 19, 2016
Updated changelogs for #5723, #5554.
@1ec5
Copy link
Contributor

1ec5 commented Jul 19, 2016

Updated the iOS and macOS changelogs in a53dfcd.

1ec5 added a commit that referenced this pull request Jul 19, 2016
Updated changelogs for #5723, #5554.

Cherry-picked from a53dfcd.
@1ec5
Copy link
Contributor

1ec5 commented Jul 19, 2016

Cherry-picked to release-ios-v3.3.0 branch as d640c22 and c261d0b.

The fix is now in both Android SDK v4.1.0 v4.1.1 and iOS SDK v3.3.1. A macOS SDK fix is coming shortly.

@bleege
Copy link
Contributor

bleege commented Jul 19, 2016

Just clarifying that it's actually Android SDK v4.1.1.

@1ec5
Copy link
Contributor

1ec5 commented Jul 19, 2016

The fix is now in macOS SDK v0.2.1 as well.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Core The cross-platform C++ core, aka mbgl crash
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants