[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[tor-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



Title: GitLab

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:

Changes:

  • dom/manifest/Manifest.sys.mjs
    ... ... @@ -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
       }
    

  • dom/manifest/test/browser_Manifest_install.js
    ... ... @@ -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");
    

  • gfx/thebes/gfxFont.cpp
    ... ... @@ -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() {
    

  • gfx/thebes/gfxFontEntry.cpp
    ... ... @@ -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
     
    

  • gfx/thebes/gfxFontEntry.h
    ... ... @@ -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,
    

  • _______________________________________________
    tor-commits mailing list
    tor-commits@xxxxxxxxxxxxxxxxxxxx
    https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits