243500 – Nullptr crash in FontCache::shouldAutoActivateFontIfNeeded

RESOLVED FIXED 243500

Nullptr crash in FontCache::shouldAutoActivateFontIfNeeded

https://bugs.webkit.org/show_bug.cgi?id=243500

Summary Nullptr crash in FontCache::shouldAutoActivateFontIfNeeded

Ahmad Saleem

Reported 2022-08-03 10:12:56 PDT

Hi Team, Came across a test case on Twitter posted by an individual, which crashes Safari and Chrome both: Tweet Link - https://twitter.com/yisibl/status/1554691218679861248?s=21&t=z_ZRMNvPNJWLKYRVdVrxAg Test Case: data:text/html;charset=UTF-8,<!doctype html> <style> @font-face { font-family: 'TestFont'; src: local(revert); } </style> <p style="font-family: TestFont">Crash</p> Chrome Bug - https://bugs.chromium.org/p/chromium/issues/detail?id=1342244 Chrome Bug Commit - https://chromium.googlesource.com/chromium/src/+/545aac9815510ec074d78e2a63887a0d59e1b9fa Appreciate if this can be fixed within Safari / Webkit as well. Thanks for your support.

Attachments
Patch (2.69 KB, patch) 2022-08-04 00:05 PDT, Ryosuke Niwa	no flags	Details Formatted Diff Diff
Patch for landing (2.64 KB, patch) 2022-08-04 11:52 PDT, Ryosuke Niwa	ews-feeder: commit-queue-	Details Formatted Diff Diff
Patch for landing (2.65 KB, patch) 2022-08-04 12:05 PDT, Ryosuke Niwa	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Simon Fraser (smfr)

Comment 1 2022-08-03 10:40:58 PDT

Reproduces. Crash I a null de-ref here: Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 WebCore 0x484558f40 WTF::HashTable<WTF::AtomString, WTF::AtomString, WTF::IdentityExtractor, WTF::DefaultHash<WTF::AtomString>, WTF::HashTraits<WTF::AtomString>, WTF::HashTraits<WTF::AtomString> >::add(WTF::AtomString const&) + 48 1 WebCore 0x4851095cf WebCore::FontCache::createFontPlatformData(WebCore::FontDescription const&, WTF::AtomString const&, WebCore::FontCreationContext const&) + 863 2 WebCore 0x485055319 WebCore::FontCache::cachedFontPlatformData(WebCore::FontDescription const&, WTF::String const&, WebCore::FontCreationContext const&, bool) + 825 3 WebCore 0x4846d3eaa WebCore::CSSFontFaceSource::load(WebCore::Document*) + 1082 4 WebCore 0x4846d33e3 WebCore::CSSFontFace::pump(WebCore::ExternalResourceDownloadPolicy) + 339 5 WebCore 0x4846d4521 WebCore::CSSFontFace::font(WebCore::FontDescription const&, bool, bool, WebCore::ExternalResourceDownloadPolicy, WebCore::FontPaletteValues const&) + 129 6 WebCore 0x48470df04 WebCore::CSSFontAccessor::font(WebCore::ExternalResourceDownloadPolicy) const + 84 7 WebCore 0x48470d8c1 WebCore::CSSSegmentedFontFace::fontRanges(WebCore::FontDescription const&, WebCore::FontPaletteValues const&) + 2353 8 WebCore 0x4846e07da WebCore::CSSFontSelector::fontRangesForFamily(WebCore::FontDescription const&, WTF::AtomString const&) + 1018 9 WebCore 0x48506395b decltype(auto) std::__1::__variant_detail::__visitation::__base::__dispatcher<0ul>::__dispatch<std::__1::__variant_detail::__visitation::__variant::__value_visitor<WTF::Visitor<WebCore::realizeNextFallback(WebCore::FontCascadeDescription const&, unsigned int&, WebCore::FontSelector*)::$_7, WebCore::realizeNextFallback(WebCore::FontCascadeDescription const&, unsigned int&, WebCore::FontSelector*)::$_8>&>&&, std::__1::__variant_detail::__base<(std::__1::__variant_detail::_Trait)1, WTF::AtomString, WebCore::FontFamilySpecificationCoreText> const&>(std::__1::__variant_detail::__visitation::__variant::__value_visitor<WTF::Visitor<WebCore::realizeNextFallback(WebCore::FontCascadeDescription const&, unsigned int&, WebCore::FontSelector*)::$_7, WebCore::realizeNextFallback(WebCore::FontCascadeDescription const&, unsigned int&, WebCore::FontSelector*)::$_8>&>&&, std::__1::__variant_detail::__base<(std::__1::__variant_detail::_Trait)1, WTF::AtomString, WebCore::FontFamilySpecificationCoreText> const&) + 75

Radar WebKit Bug Importer

Comment 2 2022-08-03 10:41:11 PDT

<rdar://problem/98062982>

Ryosuke Niwa

Comment 3 2022-08-03 23:29:21 PDT

I'm gonna be cautious and turn this into a security bug since we're looking up hash table with null string, which may or may not corrupt the hash table.

Ryosuke Niwa

Comment 4 2022-08-03 23:29:38 PDT

#0 0x00000006482747ec in WTF::StringImpl::rawHash() const at /Volumes/Data/safari/OpenSource/WebKitBuild/Debug/usr/local/include/wtf/text/StringImpl.h:336 #1 0x00000006482747c5 in WTF::StringImpl::hasHash() const at /Volumes/Data/safari/OpenSource/WebKitBuild/Debug/usr/local/include/wtf/text/StringImpl.h:339 #2 0x000000064827474d in WTF::StringImpl::existingHash() const at /Volumes/Data/safari/OpenSource/WebKitBuild/Debug/usr/local/include/wtf/text/StringImpl.h:341 #3 0x000000064827471d in WTF::AtomStringHash::hash(WTF::AtomString const&) at /Volumes/Data/safari/OpenSource/WebKitBuild/Debug/usr/local/include/wtf/text/AtomStringHash.h:39 #4 0x0000000648274065 in unsigned int WTF::IdentityHashTranslator<WTF::HashTraits<WTF::AtomString>, WTF::DefaultHash<WTF::AtomString> >::hash<WTF::AtomString>(WTF::AtomString const&) at /Volumes/Data/safari/OpenSource/WebKitBuild/Debug/usr/local/include/wtf/HashTable.h:311 #5 0x000000064ae5e7ed in WTF::HashTableAddResult<WTF::HashTableIterator<WTF::HashTable<WTF::AtomString, WTF::AtomString, WTF::IdentityExtractor, WTF::DefaultHash<WTF::AtomString>, WTF::HashTraits<WTF::AtomString>, WTF::HashTraits<WTF::AtomString> >, WTF::AtomString, WTF::AtomString, WTF::IdentityExtractor, WTF::DefaultHash<WTF::AtomString>, WTF::HashTraits<WTF::AtomString>, WTF::HashTraits<WTF::AtomString> > > WTF::HashTable<WTF::AtomString, WTF::AtomString, WTF::IdentityExtractor, WTF::DefaultHash<WTF::AtomString>, WTF::HashTraits<WTF::AtomString>, WTF::HashTraits<WTF::AtomString> >::add<WTF::IdentityHashTranslator<WTF::HashTraits<WTF::AtomString>, WTF::DefaultHash<WTF::AtomString> >, WTF::AtomString const&, WTF::AtomString const&>(WTF::AtomString const&, WTF::AtomString const&) at /Volumes/Data/safari/OpenSource/WebKitBuild/Debug/usr/local/include/wtf/HashTable.h:893 #6 0x000000064ae5e6f4 in WTF::HashTable<WTF::AtomString, WTF::AtomString, WTF::IdentityExtractor, WTF::DefaultHash<WTF::AtomString>, WTF::HashTraits<WTF::AtomString>, WTF::HashTraits<WTF::AtomString> >::add(WTF::AtomString const&) at /Volumes/Data/safari/OpenSource/WebKitBuild/Debug/usr/local/include/wtf/HashTable.h:489 #7 0x000000064ae3203b in WTF::HashSet<WTF::AtomString, WTF::DefaultHash<WTF::AtomString>, WTF::HashTraits<WTF::AtomString>, WTF::HashTableTraits>::add(WTF::AtomString const&) at /Volumes/Data/safari/OpenSource/WebKitBuild/Debug/usr/local/include/wtf/HashSet.h:267 #8 0x000000064c92f81b in WebCore::FontCache::shouldAutoActivateFontIfNeeded(WTF::AtomString const&) at /Volumes/Data/safari/OpenSource/Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:1301 #9 0x000000064c92fa00 in WebCore::FontCache::createFontPlatformData(WebCore::FontDescription const&, WTF::AtomString const&, WebCore::FontCreationContext const&) at /Volumes/Data/safari/OpenSource/Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:1324 #10 0x000000064c765409 in WebCore::FontCache::cachedFontPlatformData(WebCore::FontDescription const&, WTF::String const&, WebCore::FontCreationContext const&, bool) at /Volumes/Data/safari/OpenSource/Source/WebCore/platform/graphics/FontCache.cpp:219 #11 0x000000064c765bc6 in WebCore::FontCache::fontForFamily(WebCore::FontDescription const&, WTF::String const&, WebCore::FontCreationContext const&, bool) at /Volumes/Data/safari/OpenSource/Source/WebCore/platform/graphics/FontCache.cpp:254 #12 0x000000064b20e143 in WebCore::CSSFontFaceSource::load(WebCore::Document*) at /Volumes/Data/safari/OpenSource/Source/WebCore/css/CSSFontFaceSource.cpp:190 #13 0x000000064b20c5fe in WebCore::CSSFontFace::pump(WebCore::ExternalResourceDownloadPolicy) at /Volumes/Data/safari/OpenSource/Source/WebCore/css/CSSFontFace.cpp:590 #14 0x000000064b20e2bf in WebCore::CSSFontFace::font(WebCore::FontDescription const&, bool, bool, WebCore::ExternalResourceDownloadPolicy, WebCore::FontPaletteValues const&) at /Volumes/Data/safari/OpenSource/Source/WebCore/css/CSSFontFace.cpp:654

Ryosuke Niwa

Comment 5 2022-08-03 23:32:26 PDT

Geoff/Chris: Can this crash or some variant of it lead to a hash table corruption or not?

Ryosuke Niwa

Comment 6 2022-08-04 00:05:48 PDT

Created attachment 461394 [details] Patch

Ryosuke Niwa

Comment 7 2022-08-04 10:42:38 PDT

Chris says this is a reliable nullptr so let's reclassify it back to non-security.

Ryosuke Niwa

Comment 8 2022-08-04 10:46:52 PDT

Pull request: https://github.com/WebKit/WebKit/pull/3015

Chris Dumez

Comment 9 2022-08-04 10:58:51 PDT

Comment on attachment 461394 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=461394&action=review r=me with suggestion > Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:1301 > + return !family.isEmpty() && m_knownFamilies.add(family).isNewEntry; Would you mind doing this a little bit earlier in the function so we avoid unnecessarily doing a remove() from m_knownFamilies.remove() if at capacity?

Ryosuke Niwa

Comment 10 2022-08-04 11:01:01 PDT

Comment on attachment 461394 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=461394&action=review >> Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:1301 >> + return !family.isEmpty() && m_knownFamilies.add(family).isNewEntry; > > Would you mind doing this a little bit earlier in the function so we avoid unnecessarily doing a remove() from m_knownFamilies.remove() if at capacity? Good point. Will do.

Ryosuke Niwa

Comment 11 2022-08-04 11:52:21 PDT

Created attachment 461405 [details] Patch for landing

Ryosuke Niwa

Comment 12 2022-08-04 12:05:47 PDT

Created attachment 461406 [details] Patch for landing

EWS

Comment 13 2022-08-04 13:07:08 PDT

Committed 253127@main (689a3693a796): <https://commits.webkit.org/253127@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 461406 [details].

Sam Sneddon [:gsnedders]

Comment 14 2022-09-01 08:46:41 PDT

*** Bug 244294 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version WebKit Nightly Build

Hardware All

OS macOS 12

Product WebKit

Component Text

Assignee

Ryosuke Niwa

Reported

2022-08-03 10:12 PDT

Modified

2022-09-01 08:46 PDT History

CC List

12 users Show

URL

Keywords InRadar

Duplicates (1)

244294 View as bug list

Depends on

Blocks