CVE-2021-45931: Out-of-bounds Write (CWE-787)
value: HarfBuzz 2.9.0 has an out-of-bounds write in hb_bit_set_invertible_t::set (called from hb_sparseset_t<hb_bit_set_invertible_t>::set and hb_set_copy).
CWE description: The product writes data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 6.5
Published: 2022-01-01T01:15Z
Repository: https://github.com/harfbuzz/harfbuzz
Language: C++
Files changed: src/hb-bit-set-invertible.hh, src/hb-map.cc, src/hb-map.hh, src/hb-set.cc
Commit date: 2021-08-24 10:31:49 -0600
Commit message: [set] Make all operators null-safe again

Changed my mind.

Also for hb_map_clear().

Part of https://github.com/harfbuzz/harfbuzz/pull/3162

Diff: diff --git a/src/hb-bit-set-invertible.hh b/src/hb-bit-set-invertible.hh
index ab48239bb..f48b72fe6 100644
--- a/src/hb-bit-set-invertible.hh
+++ b/src/hb-bit-set-invertible.hh
@@ -44,13 +44,26 @@ struct hb_bit_set_invertible_t
   void fini () { s.fini (); }
   void err () { s.err (); }
   bool in_error () const { return s.in_error (); }
   explicit operator bool () const { return !is_empty (); }
 
-  void reset () { s.reset (); inverted = false; }
-  void clear () { s.clear (); inverted = false; }
-  void invert () { inverted = !inverted; }
+  void reset ()
+  {
+    s.reset ();
+    inverted = false;
+  }
+  void clear ()
+  {
+    s.clear ();
+    if (likely (s.successful))
+      inverted = false;
+  }
+  void invert ()
+  {
+    if (likely (s.successful))
+      inverted = !inverted;
+  }
 
   bool is_empty () const
   {
     hb_codepoint_t v = INVALID;
     next (&v);
@@ -114,11 +127,16 @@ struct hb_bit_set_invertible_t
   {
     hb_codepoint_t c = first - 1;
     return next (&c) && c <= last;
   }
 
-  void set (const hb_bit_set_invertible_t &other) { s.set (other.s); inverted = other.inverted; }
+  void set (const hb_bit_set_invertible_t &other)
+  {
+    s.set (other.s);
+    if (likely (s.successful))
+      inverted = other.inverted;
+  }
 
   bool is_equal (const hb_bit_set_invertible_t &other) const
   {
     if (likely (inverted == other.inverted))
       return s.is_equal (other.s);
@@ -159,11 +177,12 @@ struct hb_bit_set_invertible_t
       if (unlikely (inverted))
 	process (hb_bitwise_gt, other);
       else
 	process (hb_bitwise_lt, other);
     }
-    inverted = inverted || other.inverted;
+    if (likely (s.successful))
+      inverted = inverted || other.inverted;
   }
   void intersect (const hb_bit_set_invertible_t &other)
   {
     if (likely (inverted == other.inverted))
     {
@@ -177,11 +196,12 @@ struct hb_bit_set_invertible_t
       if (unlikely (inverted))
 	process (hb_bitwise_lt, other);
       else
 	process (hb_bitwise_gt, other);
     }
-    inverted = inverted && other.inverted;
+    if (likely (s.successful))
+      inverted = inverted && other.inverted;
   }
   void subtract (const hb_bit_set_invertible_t &other)
   {
     if (likely (inverted == other.inverted))
     {
@@ -195,16 +215,18 @@ struct hb_bit_set_invertible_t
       if (unlikely (inverted))
 	process (hb_bitwise_or, other);
       else
 	process (hb_bitwise_and, other);
     }
-    inverted = inverted && !other.inverted;
+    if (likely (s.successful))
+      inverted = inverted && !other.inverted;
   }
   void symmetric_difference (const hb_bit_set_invertible_t &other)
   {
     process (hb_bitwise_xor, other);
-    inverted = inverted ^ other.inverted;
+    if (likely (s.successful))
+      inverted = inverted ^ other.inverted;
   }
 
   bool next (hb_codepoint_t *codepoint) const
   {
     if (likely (!inverted))
diff --git a/src/hb-map.cc b/src/hb-map.cc
index 388e67886..9f1ac4284 100644
--- a/src/hb-map.cc
+++ b/src/hb-map.cc
@@ -253,13 +253,10 @@ hb_map_has (const hb_map_t *map,
  * Since: 1.7.7
  **/
 void
 hb_map_clear (hb_map_t *map)
 {
-  if (unlikely (hb_object_is_immutable (map)))
-    return;
-
   return map->clear ();
 }
 
 /**
  * hb_map_is_empty:
diff --git a/src/hb-map.hh b/src/hb-map.hh
index 751cab684..dcd5267d7 100644
--- a/src/hb-map.hh
+++ b/src/hb-map.hh
@@ -167,10 +167,12 @@ struct hb_hashmap_t
   /* Projection. */
   V operator () (K k) const { return get (k); }
 
   void clear ()
   {
+    if (unlikely (!successful)) return;
+
     if (items)
       for (auto &_ : hb_iter (items, mask + 1))
 	_.clear ();
 
     population = occupancy = 0;
diff --git a/src/hb-set.cc b/src/hb-set.cc
index f9bc85869..204dbb564 100644
--- a/src/hb-set.cc
+++ b/src/hb-set.cc
@@ -199,13 +199,11 @@ hb_set_copy (const hb_set_t *set)
  * Since: 0.9.2
  **/
 void
 hb_set_clear (hb_set_t *set)
 {
-  if (unlikely (hb_object_is_immutable (set)))
-    return;
-
+  /* Immutible-safe. */
   set->clear ();
 }
 
 /**
  * hb_set_is_empty:
@@ -366,13 +364,11 @@ hb_set_is_subset (const hb_set_t *set,
  **/
 void
 hb_set_set (hb_set_t       *set,
 	    const hb_set_t *other)
 {
-  if (unlikely (hb_object_is_immutable (set)))
-    return;
-
+  /* Immutible-safe. */
   set->set (*other);
 }
 
 /**
  * hb_set_union:
@@ -385,13 +381,11 @@ hb_set_set (hb_set_t       *set,
  **/
 void
 hb_set_union (hb_set_t       *set,
 	      const hb_set_t *other)
 {
-  if (unlikely (hb_object_is_immutable (set)))
-    return;
-
+  /* Immutible-safe. */
   set->union_ (*other);
 }
 
 /**
  * hb_set_intersect:
@@ -404,13 +398,11 @@ hb_set_union (hb_set_t       *set,
  **/
 void
 hb_set_intersect (hb_set_t       *set,
 		  const hb_set_t *other)
 {
-  if (unlikely (hb_object_is_immutable (set)))
-    return;
-
+  /* Immutible-safe. */
   set->intersect (*other);
 }
 
 /**
  * hb_set_subtract:
@@ -423,13 +415,11 @@ hb_set_intersect (hb_set_t       *set,
  **/
 void
 hb_set_subtract (hb_set_t       *set,
 		 const hb_set_t *other)
 {
-  if (unlikely (hb_object_is_immutable (set)))
-    return;
-
+  /* Immutible-safe. */
   set->subtract (*other);
 }
 
 /**
  * hb_set_symmetric_difference:
@@ -443,13 +433,11 @@ hb_set_subtract (hb_set_t       *set,
  **/
 void
 hb_set_symmetric_difference (hb_set_t       *set,
 			     const hb_set_t *other)
 {
-  if (unlikely (hb_object_is_immutable (set)))
-    return;
-
+  /* Immutible-safe. */
   set->symmetric_difference (*other);
 }
 
 /**
  * hb_set_invert:
@@ -460,13 +448,11 @@ hb_set_symmetric_difference (hb_set_t       *set,
  * Since: 3.0.0
  **/
 void
 hb_set_invert (hb_set_t *set)
 {
-  if (unlikely (hb_object_is_immutable (set)))
-    return;
-
+  /* Immutible-safe. */
   set->invert ();
 }
 
 /**
  * hb_set_get_population:

Vulnerable code:   void reset () { s.reset (); inverted = false; }
  void clear () { s.clear (); inverted = false; }
  void invert () { inverted = !inverted; }
  void set (const hb_bit_set_invertible_t &other) { s.set (other.s); inverted = other.inverted; }
    inverted = inverted || other.inverted;
    inverted = inverted && other.inverted;
    inverted = inverted && !other.inverted;
    inverted = inverted ^ other.inverted;
  if (unlikely (hb_object_is_immutable (map)))
    return;

  if (unlikely (hb_object_is_immutable (set)))
    return;

  if (unlikely (hb_object_is_immutable (set)))
    return;

  if (unlikely (hb_object_is_immutable (set)))
    return;

  if (unlikely (hb_object_is_immutable (set)))
    return;

  if (unlikely (hb_object_is_immutable (set)))
    return;

  if (unlikely (hb_object_is_immutable (set)))
    return;

  if (unlikely (hb_object_is_immutable (set)))
    return;

Fixed code:   void reset ()
  {
    s.reset ();
    inverted = false;
  }
  void clear ()
  {
    s.clear ();
    if (likely (s.successful))
      inverted = false;
  }
  void invert ()
  {
    if (likely (s.successful))
      inverted = !inverted;
  }
  void set (const hb_bit_set_invertible_t &other)
  {
    s.set (other.s);
    if (likely (s.successful))
      inverted = other.inverted;
  }
    if (likely (s.successful))
      inverted = inverted || other.inverted;
    if (likely (s.successful))
      inverted = inverted && other.inverted;
    if (likely (s.successful))
      inverted = inverted && !other.inverted;
    if (likely (s.successful))
      inverted = inverted ^ other.inverted;
    if (unlikely (!successful)) return;

  /* Immutible-safe. */
  /* Immutible-safe. */
  /* Immutible-safe. */
  /* Immutible-safe. */
  /* Immutible-safe. */
  /* Immutible-safe. */
  /* Immutible-safe. */