Skip to content

Commit

Permalink
Merge #46: Fix invalid pointer arithmetic in Hash (google#1222)
Browse files Browse the repository at this point in the history
a8844b2 Fix invalid pointer arithmetic in Hash (google#1222) (David Benjamin)

Pull request description:

  It is UB to exceed the bounds of the buffer when doing pointer arithemetic. That means the following is not a valid bounds check:

      if (start + 4 <= limit)

  Because if we were at the end of the buffer, we wouldn't be allowed to add 4 anyway. Instead, this must be written as:

      if (limit - start >= 4)

  Basic forms of this issue are flagged by UBSan. If building with -fsanitize=undefined, the following test trips an error:

      [ RUN      ] HASH.SignedUnsignedIssue
      .../leveldb/util/hash.cc:30:15: runtime error: applying non-zero offset 4 to null pointer
      SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/local/google/home/davidben/leveldb/util/hash.cc:30:15 in
      [       OK ] HASH.SignedUnsignedIssue (1 ms)

  (cherry picked from commit 578eeb7)

ACKs for top commit:
  l0rinc:
    ACK a8844b2
  fanquake:
    ACK a8844b2

Tree-SHA512: 4e3e7aa680ec0a7ed759dd47318876e78cc04bd65456b7b0e41f7c29f1f70cf0f9568b7f32056bedae8492ee7fe5510f25d74a48a27518185248c5c34e54fa2f
  • Loading branch information
fanquake committed Jan 16, 2025
2 parents 5966981 + a8844b2 commit 04b5790
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion util/hash.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ uint32_t Hash(const char* data, size_t n, uint32_t seed) {
uint32_t h = seed ^ (n * m);

// Pick up four bytes at a time
while (data + 4 <= limit) {
while (limit - data >= 4) {
uint32_t w = DecodeFixed32(data);
data += 4;
h += w;
Expand Down

0 comments on commit 04b5790

Please sign in to comment.