ma1 pushed to branch tor-browser-115.11.0esr-13.0-1 at The Tor Project / Applications / Tor Browser
Commits:
-
a52dc31b
by Nuohan Li at 2024-05-09T10:55:04+00:00
-
88affd4a
by Jonathan Kew at 2024-05-09T10:55:04+00:00
-
fc0ee191
by Jonathan Kew at 2024-05-09T10:55:04+00:00
5 changed files:
- dom/manifest/Manifest.sys.mjs
- dom/manifest/test/browser_Manifest_install.js
- gfx/thebes/gfxFont.cpp
- gfx/thebes/gfxFontEntry.cpp
- gfx/thebes/gfxFontEntry.h
Changes:
| ... | ... | @@ -29,11 +29,11 @@ ChromeUtils.defineESModuleGetters(lazy, { |
| 29 | 29 | * @note The generated hash is returned in base64 form. Mind the fact base64
|
| 30 | 30 | * is case-sensitive if you are going to reuse this code.
|
| 31 | 31 | */
|
| 32 | -function generateHash(aString) {
|
|
| 32 | +function generateHash(aString, hashAlg) {
|
|
| 33 | 33 | const cryptoHash = Cc["@mozilla.org/security/hash;1"].createInstance(
|
| 34 | 34 | Ci.nsICryptoHash
|
| 35 | 35 | );
|
| 36 | - cryptoHash.init(Ci.nsICryptoHash.MD5);
|
|
| 36 | + cryptoHash.init(hashAlg);
|
|
| 37 | 37 | const stringStream = Cc[
|
| 38 | 38 | "@mozilla.org/io/string-input-stream;1"
|
| 39 | 39 | ].createInstance(Ci.nsIStringInputStream);
|
| ... | ... | @@ -66,11 +66,39 @@ class Manifest { |
| 66 | 66 | this._manifestUrl = manifestUrl;
|
| 67 | 67 | // The key for this is the manifests URL that is required to be unique.
|
| 68 | 68 | // However arbitrary urls are not safe file paths so lets hash it.
|
| 69 | - const fileName = generateHash(manifestUrl) + ".json";
|
|
| 70 | - this._path = PathUtils.join(MANIFESTS_DIR, fileName);
|
|
| 69 | + const filename =
|
|
| 70 | + generateHash(manifestUrl, Ci.nsICryptoHash.SHA256) + ".json";
|
|
| 71 | + this._path = PathUtils.join(MANIFESTS_DIR, filename);
|
|
| 71 | 72 | this.browser = browser;
|
| 72 | 73 | }
|
| 73 | 74 | |
| 75 | + /**
|
|
| 76 | + * See Bug 1871109
|
|
| 77 | + * This function is called at the beginning of initialize() to check if a given
|
|
| 78 | + * manifest has MD5 based filename, if so we remove it and migrate the content to
|
|
| 79 | + * a new file with SHA256 based name.
|
|
| 80 | + * This is done due to security concern, as MD5 is an outdated hashing algorithm and
|
|
| 81 | + * shouldn't be used anymore
|
|
| 82 | + */
|
|
| 83 | + async removeMD5BasedFilename() {
|
|
| 84 | + const filenameMD5 =
|
|
| 85 | + generateHash(this._manifestUrl, Ci.nsICryptoHash.MD5) + ".json";
|
|
| 86 | + const MD5Path = PathUtils.join(MANIFESTS_DIR, filenameMD5);
|
|
| 87 | + try {
|
|
| 88 | + await IOUtils.copy(MD5Path, this._path, { noOverwrite: true });
|
|
| 89 | + } catch (error) {
|
|
| 90 | + // we are ignoring the failures returned from copy as it should not stop us from
|
|
| 91 | + // installing a new manifest
|
|
| 92 | + }
|
|
| 93 | + |
|
| 94 | + // Remove the old MD5 based file unconditionally to ensure it's no longer used
|
|
| 95 | + try {
|
|
| 96 | + await IOUtils.remove(MD5Path);
|
|
| 97 | + } catch {
|
|
| 98 | + // ignore the error in case MD5 based file does not exist
|
|
| 99 | + }
|
|
| 100 | + }
|
|
| 101 | + |
|
| 74 | 102 | get browser() {
|
| 75 | 103 | return this._browser;
|
| 76 | 104 | }
|
| ... | ... | @@ -80,6 +108,7 @@ class Manifest { |
| 80 | 108 | }
|
| 81 | 109 | |
| 82 | 110 | async initialize() {
|
| 111 | + await this.removeMD5BasedFilename();
|
|
| 83 | 112 | this._store = new lazy.JSONFile({ path: this._path, saveDelayMs: 100 });
|
| 84 | 113 | await this._store.load();
|
| 85 | 114 | }
|
| ... | ... | @@ -23,18 +23,59 @@ function makeTestURL() { |
| 23 | 23 | return url.href;
|
| 24 | 24 | }
|
| 25 | 25 | |
| 26 | +function generateHash(aString, hashAlg) {
|
|
| 27 | + const cryptoHash = Cc["@mozilla.org/security/hash;1"].createInstance(
|
|
| 28 | + Ci.nsICryptoHash
|
|
| 29 | + );
|
|
| 30 | + cryptoHash.init(hashAlg);
|
|
| 31 | + const stringStream = Cc[
|
|
| 32 | + "@mozilla.org/io/string-input-stream;1"
|
|
| 33 | + ].createInstance(Ci.nsIStringInputStream);
|
|
| 34 | + stringStream.data = aString;
|
|
| 35 | + cryptoHash.updateFromStream(stringStream, -1);
|
|
| 36 | + // base64 allows the '/' char, but we can't use it for filenames.
|
|
| 37 | + return cryptoHash.finish(true).replace(/\//g, "-");
|
|
| 38 | +}
|
|
| 39 | + |
|
| 40 | +const MANIFESTS_DIR = PathUtils.join(PathUtils.profileDir, "manifests");
|
|
| 41 | + |
|
| 26 | 42 | add_task(async function () {
|
| 27 | 43 | const tabOptions = { gBrowser, url: makeTestURL() };
|
| 28 | 44 | |
| 45 | + const filenameMD5 = generateHash(manifestUrl, Ci.nsICryptoHash.MD5) + ".json";
|
|
| 46 | + const filenameSHA =
|
|
| 47 | + generateHash(manifestUrl, Ci.nsICryptoHash.SHA256) + ".json";
|
|
| 48 | + const manifestMD5Path = PathUtils.join(MANIFESTS_DIR, filenameMD5);
|
|
| 49 | + const manifestSHAPath = PathUtils.join(MANIFESTS_DIR, filenameSHA);
|
|
| 50 | + |
|
| 29 | 51 | await BrowserTestUtils.withNewTab(tabOptions, async function (browser) {
|
| 30 | - let manifest = await Manifests.getManifest(browser, manifestUrl);
|
|
| 31 | - is(manifest.installed, false, "We haven't installed this manifest yet");
|
|
| 52 | + let tmpManifest = await Manifests.getManifest(browser, manifestUrl);
|
|
| 53 | + is(tmpManifest.installed, false, "We haven't installed this manifest yet");
|
|
| 54 | + |
|
| 55 | + await tmpManifest.install();
|
|
| 32 | 56 | |
| 57 | + // making sure the manifest is actually installed before proceeding
|
|
| 58 | + await tmpManifest._store._save();
|
|
| 59 | + await IOUtils.move(tmpManifest.path, manifestMD5Path);
|
|
| 60 | + |
|
| 61 | + let exists = await IOUtils.exists(tmpManifest.path);
|
|
| 62 | + is(
|
|
| 63 | + exists,
|
|
| 64 | + false,
|
|
| 65 | + "Manually moved manifest from SHA256 based path to MD5 based path"
|
|
| 66 | + );
|
|
| 67 | + Manifests.manifestObjs.delete(manifestUrl);
|
|
| 68 | + |
|
| 69 | + let manifest = await Manifests.getManifest(browser, manifestUrl);
|
|
| 33 | 70 | await manifest.install(browser);
|
| 34 | 71 | is(manifest.name, "hello World", "Manifest has correct name");
|
| 35 | 72 | is(manifest.installed, true, "Manifest is installed");
|
| 36 | 73 | is(manifest.url, manifestUrl, "has correct url");
|
| 37 | 74 | is(manifest.browser, browser, "has correct browser");
|
| 75 | + is(manifest.path, manifestSHAPath, "has correct path");
|
|
| 76 | + |
|
| 77 | + exists = await IOUtils.exists(manifestMD5Path);
|
|
| 78 | + is(exists, false, "MD5 based manifest removed");
|
|
| 38 | 79 | |
| 39 | 80 | manifest = await Manifests.getManifest(browser, manifestUrl);
|
| 40 | 81 | is(manifest.installed, true, "New instances are installed");
|
| ... | ... | @@ -952,6 +952,10 @@ gfxFont::gfxFont(const RefPtr<UnscaledFont>& aUnscaledFont, |
| 952 | 952 | }
|
| 953 | 953 | |
| 954 | 954 | mKerningSet = HasFeatureSet(HB_TAG('k', 'e', 'r', 'n'), mKerningEnabled);
|
| 955 | + |
|
| 956 | + // Ensure the gfxFontEntry's unitsPerEm and extents fields are initialized,
|
|
| 957 | + // so that GetFontExtents can use them without risk of races.
|
|
| 958 | + Unused << mFontEntry->UnitsPerEm();
|
|
| 955 | 959 | }
|
| 956 | 960 | |
| 957 | 961 | gfxFont::~gfxFont() {
|
| ... | ... | @@ -262,14 +262,22 @@ already_AddRefed<gfxFont> gfxFontEntry::FindOrMakeFont( |
| 262 | 262 | }
|
| 263 | 263 | |
| 264 | 264 | uint16_t gfxFontEntry::UnitsPerEm() {
|
| 265 | + {
|
|
| 266 | + AutoReadLock lock(mLock);
|
|
| 267 | + if (mUnitsPerEm) {
|
|
| 268 | + return mUnitsPerEm;
|
|
| 269 | + }
|
|
| 270 | + }
|
|
| 271 | + |
|
| 272 | + AutoTable headTable(this, TRUETYPE_TAG('h', 'e', 'a', 'd'));
|
|
| 273 | + AutoWriteLock lock(mLock);
|
|
| 274 | + |
|
| 265 | 275 | if (!mUnitsPerEm) {
|
| 266 | - AutoTable headTable(this, TRUETYPE_TAG('h', 'e', 'a', 'd'));
|
|
| 267 | 276 | if (headTable) {
|
| 268 | 277 | uint32_t len;
|
| 269 | 278 | const HeadTable* head =
|
| 270 | 279 | reinterpret_cast<const HeadTable*>(hb_blob_get_data(headTable, &len));
|
| 271 | 280 | if (len >= sizeof(HeadTable)) {
|
| 272 | - mUnitsPerEm = head->unitsPerEm;
|
|
| 273 | 281 | if (int16_t(head->xMax) > int16_t(head->xMin) &&
|
| 274 | 282 | int16_t(head->yMax) > int16_t(head->yMin)) {
|
| 275 | 283 | mXMin = head->xMin;
|
| ... | ... | @@ -277,6 +285,7 @@ uint16_t gfxFontEntry::UnitsPerEm() { |
| 277 | 285 | mXMax = head->xMax;
|
| 278 | 286 | mYMax = head->yMax;
|
| 279 | 287 | }
|
| 288 | + mUnitsPerEm = head->unitsPerEm;
|
|
| 280 | 289 | }
|
| 281 | 290 | }
|
| 282 | 291 | |
| ... | ... | @@ -286,12 +295,13 @@ uint16_t gfxFontEntry::UnitsPerEm() { |
| 286 | 295 | mUnitsPerEm = kInvalidUPEM;
|
| 287 | 296 | }
|
| 288 | 297 | }
|
| 298 | + |
|
| 289 | 299 | return mUnitsPerEm;
|
| 290 | 300 | }
|
| 291 | 301 | |
| 292 | 302 | bool gfxFontEntry::HasSVGGlyph(uint32_t aGlyphId) {
|
| 293 | - NS_ASSERTION(mSVGInitialized,
|
|
| 294 | - "SVG data has not yet been loaded. TryGetSVGData() first.");
|
|
| 303 | + MOZ_ASSERT(mSVGInitialized,
|
|
| 304 | + "SVG data has not yet been loaded. TryGetSVGData() first.");
|
|
| 295 | 305 | return GetSVGGlyphs()->HasSVGGlyph(aGlyphId);
|
| 296 | 306 | }
|
| 297 | 307 | |
| ... | ... | @@ -309,8 +319,8 @@ bool gfxFontEntry::GetSVGGlyphExtents(DrawTarget* aDrawTarget, |
| 309 | 319 | |
| 310 | 320 | void gfxFontEntry::RenderSVGGlyph(gfxContext* aContext, uint32_t aGlyphId,
|
| 311 | 321 | SVGContextPaint* aContextPaint) {
|
| 312 | - NS_ASSERTION(mSVGInitialized,
|
|
| 313 | - "SVG data has not yet been loaded. TryGetSVGData() first.");
|
|
| 322 | + MOZ_ASSERT(mSVGInitialized,
|
|
| 323 | + "SVG data has not yet been loaded. TryGetSVGData() first.");
|
|
| 314 | 324 | GetSVGGlyphs()->RenderGlyph(aContext, aGlyphId, aContextPaint);
|
| 315 | 325 | }
|
| 316 | 326 | |
| ... | ... | @@ -467,8 +477,9 @@ hb_blob_t* gfxFontEntry::FontTableHashEntry::ShareTableAndGetBlob( |
| 467 | 477 | HB_MEMORY_MODE_READONLY, mSharedBlobData, DeleteFontTableBlobData);
|
| 468 | 478 | if (mBlob == hb_blob_get_empty()) {
|
| 469 | 479 | // The FontTableBlobData was destroyed during hb_blob_create().
|
| 470 | - // The (empty) blob is still be held in the hashtable with a strong
|
|
| 480 | + // The (empty) blob will still be held in the hashtable with a strong
|
|
| 471 | 481 | // reference.
|
| 482 | + mSharedBlobData = nullptr;
|
|
| 472 | 483 | return hb_blob_reference(mBlob);
|
| 473 | 484 | }
|
| 474 | 485 |
| ... | ... | @@ -538,6 +538,9 @@ class gfxFontEntry { |
| 538 | 538 | |
| 539 | 539 | mozilla::gfx::Rect GetFontExtents(float aFUnitScaleFactor) const {
|
| 540 | 540 | // Flip the y-axis here to match the orientation of Gecko's coordinates.
|
| 541 | + // We don't need to take a lock here because the min/max fields are inert
|
|
| 542 | + // after initialization, and we make sure to initialize them at gfxFont-
|
|
| 543 | + // creation time.
|
|
| 541 | 544 | return mozilla::gfx::Rect(float(mXMin) * aFUnitScaleFactor,
|
| 542 | 545 | float(-mYMax) * aFUnitScaleFactor,
|
| 543 | 546 | float(mXMax - mXMin) * aFUnitScaleFactor,
|