dom/manifest/Manifest.sys.mjs

@@ -29,11 +29,11 @@ ChromeUtils.defineESModuleGetters(lazy, {
  * @note The generated hash is returned in base64 form.  Mind the fact base64
  * is case-sensitive if you are going to reuse this code.
  */
-function generateHash(aString) {
+function generateHash(aString, hashAlg) {
   const cryptoHash = Cc["@mozilla.org/security/hash;1"].createInstance(
     Ci.nsICryptoHash
   );
-  cryptoHash.init(Ci.nsICryptoHash.MD5);
+  cryptoHash.init(hashAlg);
   const stringStream = Cc[
     "@mozilla.org/io/string-input-stream;1"
   ].createInstance(Ci.nsIStringInputStream);
@@ -66,11 +66,39 @@ class Manifest {
     this._manifestUrl = manifestUrl;
     // The key for this is the manifests URL that is required to be unique.
     // However arbitrary urls are not safe file paths so lets hash it.
-    const fileName = generateHash(manifestUrl) + ".json";
-    this._path = PathUtils.join(MANIFESTS_DIR, fileName);
+    const filename =
+      generateHash(manifestUrl, Ci.nsICryptoHash.SHA256) + ".json";
+    this._path = PathUtils.join(MANIFESTS_DIR, filename);
     this.browser = browser;
   }
 
+  /**
+   * See Bug 1871109
+   * This function is called at the beginning of initialize() to check if a given
+   * manifest has MD5 based filename, if so we remove it and migrate the content to
+   * a new file with SHA256 based name.
+   * This is done due to security concern, as MD5 is an outdated hashing algorithm and
+   * shouldn't be used anymore
+   */
+  async removeMD5BasedFilename() {
+    const filenameMD5 =
+      generateHash(this._manifestUrl, Ci.nsICryptoHash.MD5) + ".json";
+    const MD5Path = PathUtils.join(MANIFESTS_DIR, filenameMD5);
+    try {
+      await IOUtils.copy(MD5Path, this._path, { noOverwrite: true });
+    } catch (error) {
+      // we are ignoring the failures returned from copy as it should not stop us from
+      // installing a new manifest
+    }
+
+    // Remove the old MD5 based file unconditionally to ensure it's no longer used
+    try {
+      await IOUtils.remove(MD5Path);
+    } catch {
+      // ignore the error in case MD5 based file does not exist
+    }
+  }
+
   get browser() {
     return this._browser;
   }
@@ -80,6 +108,7 @@ class Manifest {
   }
 
   async initialize() {
+    await this.removeMD5BasedFilename();
     this._store = new lazy.JSONFile({ path: this._path, saveDelayMs: 100 });
     await this._store.load();
   }

dom/manifest/test/browser_Manifest_install.js

@@ -23,18 +23,59 @@ function makeTestURL() {
   return url.href;
 }
 
+function generateHash(aString, hashAlg) {
+  const cryptoHash = Cc["@mozilla.org/security/hash;1"].createInstance(
+    Ci.nsICryptoHash
+  );
+  cryptoHash.init(hashAlg);
+  const stringStream = Cc[
+    "@mozilla.org/io/string-input-stream;1"
+  ].createInstance(Ci.nsIStringInputStream);
+  stringStream.data = aString;
+  cryptoHash.updateFromStream(stringStream, -1);
+  // base64 allows the '/' char, but we can't use it for filenames.
+  return cryptoHash.finish(true).replace(/\//g, "-");
+}
+
+const MANIFESTS_DIR = PathUtils.join(PathUtils.profileDir, "manifests");
+
 add_task(async function () {
   const tabOptions = { gBrowser, url: makeTestURL() };
 
+  const filenameMD5 = generateHash(manifestUrl, Ci.nsICryptoHash.MD5) + ".json";
+  const filenameSHA =
+    generateHash(manifestUrl, Ci.nsICryptoHash.SHA256) + ".json";
+  const manifestMD5Path = PathUtils.join(MANIFESTS_DIR, filenameMD5);
+  const manifestSHAPath = PathUtils.join(MANIFESTS_DIR, filenameSHA);
+
   await BrowserTestUtils.withNewTab(tabOptions, async function (browser) {
-    let manifest = await Manifests.getManifest(browser, manifestUrl);
-    is(manifest.installed, false, "We haven't installed this manifest yet");
+    let tmpManifest = await Manifests.getManifest(browser, manifestUrl);
+    is(tmpManifest.installed, false, "We haven't installed this manifest yet");
+
+    await tmpManifest.install();
 
+    // making sure the manifest is actually installed before proceeding
+    await tmpManifest._store._save();
+    await IOUtils.move(tmpManifest.path, manifestMD5Path);
+
+    let exists = await IOUtils.exists(tmpManifest.path);
+    is(
+      exists,
+      false,
+      "Manually moved manifest from SHA256 based path to MD5 based path"
+    );
+    Manifests.manifestObjs.delete(manifestUrl);
+
+    let manifest = await Manifests.getManifest(browser, manifestUrl);
     await manifest.install(browser);
     is(manifest.name, "hello World", "Manifest has correct name");
     is(manifest.installed, true, "Manifest is installed");
     is(manifest.url, manifestUrl, "has correct url");
     is(manifest.browser, browser, "has correct browser");
+    is(manifest.path, manifestSHAPath, "has correct path");
+
+    exists = await IOUtils.exists(manifestMD5Path);
+    is(exists, false, "MD5 based manifest removed");
 
     manifest = await Manifests.getManifest(browser, manifestUrl);
     is(manifest.installed, true, "New instances are installed");

gfx/thebes/gfxFont.cpp

@@ -952,6 +952,10 @@ gfxFont::gfxFont(const RefPtr<UnscaledFont>& aUnscaledFont,
   }
 
   mKerningSet = HasFeatureSet(HB_TAG('k', 'e', 'r', 'n'), mKerningEnabled);
+
+  // Ensure the gfxFontEntry's unitsPerEm and extents fields are initialized,
+  // so that GetFontExtents can use them without risk of races.
+  Unused << mFontEntry->UnitsPerEm();
 }
 
 gfxFont::~gfxFont() {

gfx/thebes/gfxFontEntry.cpp

@@ -262,14 +262,22 @@ already_AddRefed<gfxFont> gfxFontEntry::FindOrMakeFont(
 }
 
 uint16_t gfxFontEntry::UnitsPerEm() {
+  {
+    AutoReadLock lock(mLock);
+    if (mUnitsPerEm) {
+      return mUnitsPerEm;
+    }
+  }
+
+  AutoTable headTable(this, TRUETYPE_TAG('h', 'e', 'a', 'd'));
+  AutoWriteLock lock(mLock);
+
   if (!mUnitsPerEm) {
-    AutoTable headTable(this, TRUETYPE_TAG('h', 'e', 'a', 'd'));
     if (headTable) {
       uint32_t len;
       const HeadTable* head =
           reinterpret_cast<const HeadTable*>(hb_blob_get_data(headTable, &len));
       if (len >= sizeof(HeadTable)) {
-        mUnitsPerEm = head->unitsPerEm;
         if (int16_t(head->xMax) > int16_t(head->xMin) &&
             int16_t(head->yMax) > int16_t(head->yMin)) {
           mXMin = head->xMin;
@@ -277,6 +285,7 @@ uint16_t gfxFontEntry::UnitsPerEm() {
           mXMax = head->xMax;
           mYMax = head->yMax;
         }
+        mUnitsPerEm = head->unitsPerEm;
       }
     }
 
@@ -286,12 +295,13 @@ uint16_t gfxFontEntry::UnitsPerEm() {
       mUnitsPerEm = kInvalidUPEM;
     }
   }
+
   return mUnitsPerEm;
 }
 
 bool gfxFontEntry::HasSVGGlyph(uint32_t aGlyphId) {
-  NS_ASSERTION(mSVGInitialized,
-               "SVG data has not yet been loaded. TryGetSVGData() first.");
+  MOZ_ASSERT(mSVGInitialized,
+             "SVG data has not yet been loaded. TryGetSVGData() first.");
   return GetSVGGlyphs()->HasSVGGlyph(aGlyphId);
 }
 
@@ -309,8 +319,8 @@ bool gfxFontEntry::GetSVGGlyphExtents(DrawTarget* aDrawTarget,
 
 void gfxFontEntry::RenderSVGGlyph(gfxContext* aContext, uint32_t aGlyphId,
                                   SVGContextPaint* aContextPaint) {
-  NS_ASSERTION(mSVGInitialized,
-               "SVG data has not yet been loaded. TryGetSVGData() first.");
+  MOZ_ASSERT(mSVGInitialized,
+             "SVG data has not yet been loaded. TryGetSVGData() first.");
   GetSVGGlyphs()->RenderGlyph(aContext, aGlyphId, aContextPaint);
 }
 
@@ -467,8 +477,9 @@ hb_blob_t* gfxFontEntry::FontTableHashEntry::ShareTableAndGetBlob(
       HB_MEMORY_MODE_READONLY, mSharedBlobData, DeleteFontTableBlobData);
   if (mBlob == hb_blob_get_empty()) {
     // The FontTableBlobData was destroyed during hb_blob_create().
-    // The (empty) blob is still be held in the hashtable with a strong
+    // The (empty) blob will still be held in the hashtable with a strong
     // reference.
+    mSharedBlobData = nullptr;
     return hb_blob_reference(mBlob);
   }
 

gfx/thebes/gfxFontEntry.h

@@ -538,6 +538,9 @@ class gfxFontEntry {
 
   mozilla::gfx::Rect GetFontExtents(float aFUnitScaleFactor) const {
     // Flip the y-axis here to match the orientation of Gecko's coordinates.
+    // We don't need to take a lock here because the min/max fields are inert
+    // after initialization, and we make sure to initialize them at gfxFont-
+    // creation time.
     return mozilla::gfx::Rect(float(mXMin) * aFUnitScaleFactor,
                               float(-mYMax) * aFUnitScaleFactor,
                               float(mXMax - mXMin) * aFUnitScaleFactor,