Bug 261508

Summary: Loading disallowed FontFace from ArrayBuffer in Lockdown Mode doesn't reject promise
Product: WebKit Reporter: pajowu
Component: TextAssignee: Nobody <webkit-unassigned>
Status: NEW    
Severity: Normal CC: bfulgham, mmaxfield, ntim, pajowu, vitor.roriz, webkit-bug-importer, wenson_hsieh, wilander
Priority: P2 Keywords: InRadar
Version: Safari 16   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Minimal Reproducing Example none

pajowu
Reported 2023-09-13 06:24:07 PDT
Created attachment 467671 [details] Minimal Reproducing Example The CSS Font Loading API can be used to load fonts via JS using the `FontFace` constructor. The constructor is passed a source, which can either be a css string or `BinaryData` (an ArrayBuffer or ArrayBufferView). According to the spec[1], if BinaryData is passed, the data is directly stored in the FontFaces internal `[[Data]]` slot and then parsed asynchronously. After this parsing finished, the fonts `loading` promise shall be resolved (either fulfilled if the parsing was successful or rejected otherwise). According to the spec, the `load()` method of a `FontFace` is a no-op if the font-face is constructed from BinaryData[2]. In conclusion: According to the spec, if a FontFace is constructed from BinaryData, its `loaded` property can be used immediately without calling the `load()` method and is guaranteed to be resolved. However, when loading a custom FontFace from BinaryData in Lockdown Mode, the `loaded` promise is never resolved until `load()` is called. A simple example to reproduce this is attached as `mre.html`. The page should show "font.loaded fulfilled" or "font.loaded rejected" almost immediately after loading. However in current Safari, it is stock on `Loading font ...` forever. Adding a `font.load()` at the end fixes this, making the promise fail as expected immediately. ---- The following is educated speculation, as I'm not familiar with the webkit codebase. The problem seems to be the code in [3]/[4], which checks if the loaded font is within a number of allow-listed webfonts[5] and otherwise returns. My impression is that instead of returning empty, this should reject the `loaded` promise (`return Exception { NetworkError }`) [1]: https://www.w3.org/TR/css-font-loading/#font-face-constructor [2]: https://www.w3.org/TR/css-font-loading/#font-face-load [3]: https://github.com/WebKit/WebKit/blob/d68d5c148eb3616448ae8d8e28408c0340823181/Source/WebCore/css/FontFace.cpp#L88-L90 [3]: https://github.com/WebKit/WebKit/blob/d68d5c148eb3616448ae8d8e28408c0340823181/Source/WebCore/css/FontFace.cpp#L82-L83 [5]: https://github.com/WebKit/WebKit/blob/main/Source/WebCore/loader/cache/AllowedFonts.cpp#L41
Attachments
Minimal Reproducing Example (2.48 KB, text/html)
2023-09-13 06:24 PDT, pajowu 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
pajowu
Comment 1 2023-09-13 06:30:35 PDT
Comment on attachment 467671 [details] Minimal Reproducing Example The mre should of course be the following (only change is the `font.load()` at the end of the file commented out) ><html> > <body> > <div id="status">Page is loading...</div> > </body> > <script> > // Taken from https://github.com/mozilla/pdf.js/blob/920e51a1e6050ed9b8ddad51a3fb0a39dc4fa5ee/src/display/font_loader.js#L228 > // This is a CFF font with 1 glyph for '.' that fills its entire width > // and height. > const testFont = atob( > "T1RUTwALAIAAAwAwQ0ZGIDHtZg4AAAOYAAAAgUZGVE1lkzZwAAAEHAAAABxHREVGABQA" + > "FQAABDgAAAAeT1MvMlYNYwkAAAEgAAAAYGNtYXABDQLUAAACNAAAAUJoZWFk/xVFDQAA" + > "ALwAAAA2aGhlYQdkA+oAAAD0AAAAJGhtdHgD6AAAAAAEWAAAAAZtYXhwAAJQAAAAARgA" + > "AAAGbmFtZVjmdH4AAAGAAAAAsXBvc3T/hgAzAAADeAAAACAAAQAAAAEAALZRFsRfDzz1" + > "AAsD6AAAAADOBOTLAAAAAM4KHDwAAAAAA+gDIQAAAAgAAgAAAAAAAAABAAADIQAAAFoD" + > "6AAAAAAD6AABAAAAAAAAAAAAAAAAAAAAAQAAUAAAAgAAAAQD6AH0AAUAAAKKArwAAACM" + > "AooCvAAAAeAAMQECAAACAAYJAAAAAAAAAAAAAQAAAAAAAAAAAAAAAFBmRWQAwAAuAC4D" + > "IP84AFoDIQAAAAAAAQAAAAAAAAAAACAAIAABAAAADgCuAAEAAAAAAAAAAQAAAAEAAAAA" + > "AAEAAQAAAAEAAAAAAAIAAQAAAAEAAAAAAAMAAQAAAAEAAAAAAAQAAQAAAAEAAAAAAAUA" + > "AQAAAAEAAAAAAAYAAQAAAAMAAQQJAAAAAgABAAMAAQQJAAEAAgABAAMAAQQJAAIAAgAB" + > "AAMAAQQJAAMAAgABAAMAAQQJAAQAAgABAAMAAQQJAAUAAgABAAMAAQQJAAYAAgABWABY" + > "AAAAAAAAAwAAAAMAAAAcAAEAAAAAADwAAwABAAAAHAAEACAAAAAEAAQAAQAAAC7//wAA" + > "AC7////TAAEAAAAAAAABBgAAAQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" + > "AAAAAAAAAAAAAAAAAAEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" + > "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" + > "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" + > "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" + > "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAMAAAAAAAD/gwAyAAAAAQAAAAAAAAAAAAAAAAAA" + > "AAABAAQEAAEBAQJYAAEBASH4DwD4GwHEAvgcA/gXBIwMAYuL+nz5tQXkD5j3CBLnEQAC" + > "AQEBIVhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYAAABAQAADwACAQEEE/t3" + > "Dov6fAH6fAT+fPp8+nwHDosMCvm1Cvm1DAz6fBQAAAAAAAABAAAAAMmJbzEAAAAAzgTj" + > "FQAAAADOBOQpAAEAAAAAAAAADAAUAAQAAAABAAAAAgABAAAAAAAAAAAD6AAAAAAAAA==" > ); > const fontBuffer = Uint8Array.from(testFont, (c) => c.charCodeAt(0)); > const statusElem = document.getElementById("status"); > > const font = new FontFace("test", fontBuffer); > statusElem.innerText = "Loading font ..."; > > font.loaded.then( > () => (statusElem.innerText = "font.loaded fulfilled"), > () => (statusElem.innerText = "font.loaded rejected") > ); > // font.load(); > </script> ></html>
Radar WebKit Bug Importer
Comment 2 2023-09-13 07:42:31 PDT
<rdar://problem/115429609>
Note You need to log in before you can comment on or make changes to this bug.