[tbb-commits] [Git][tpo/applications/tor-browser][tor-browser-115.11.0esr-13.0-1] 3 commits: Bug 1871109 - generateHash in Manifest.sys.mjs should use sha256 r=peterv, a=dmeehan
ma1 (@ma1)
git at gitlab.torproject.org
Thu May 9 10:56:28 UTC 2024
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
Bug 1871109 - generateHash in Manifest.sys.mjs should use sha256 r=peterv, a=dmeehan
Differential Revision: https://phabricator.services.mozilla.com/D204928
- - - - -
88affd4a by Jonathan Kew at 2024-05-09T10:55:04+00:00
Bug 1890204 - Ensure font entry's unitsPerEm and font extents are initialized when gfxFont is created. r=gfx-reviewers,lsalzman
This means that by the time we potentially call GetFontExtents() when drawing,
the extents fields are guaranteed to have been been initialized, and there's no
risk of the (read-only) access here racing with setting them in UnitsPerEm().
Differential Revision: https://phabricator.services.mozilla.com/D206920
- - - - -
fc0ee191 by Jonathan Kew at 2024-05-09T10:55:04+00:00
Bug 1893891 - Clear mSharedBlobData if blob creation failed. a=dmeehan
Original Revision: https://phabricator.services.mozilla.com/D208983
Differential Revision: https://phabricator.services.mozilla.com/D209209
- - - - -
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:
=====================================
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,
View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/10474e51a1f444c4bd7a2e1e92783014916bab22...fc0ee191276e8ae2136967b66fad3a7c8879c17a
--
This project does not include diff previews in email notifications.
View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/10474e51a1f444c4bd7a2e1e92783014916bab22...fc0ee191276e8ae2136967b66fad3a7c8879c17a
You're receiving this email because of your account on gitlab.torproject.org.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.torproject.org/pipermail/tbb-commits/attachments/20240509/8affae7d/attachment-0001.htm>
More information about the tbb-commits
mailing list