# HG changeset patch # User Augie Fackler # Date 2018-06-13 14:24:44 # Node ID 1fb2510cf8c81f2e26ec49054fdbec67ec56cba0 # Parent 74b4a54002ecb2059612d5e69ba204d05c818542 bitmanipulation: fix undefined behavior in bit shift in getbe32 OSS-Fuzz caught this in its ubsan mode[0]. I'm not worried about a security issue here because in practice this should work out the way we naively expected, we're just making things explicit to the compiler with the casts. 0: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=8876 Differential Revision: https://phab.mercurial-scm.org/D3729 diff --git a/contrib/fuzz/mpatch_corpus.py b/contrib/fuzz/mpatch_corpus.py --- a/contrib/fuzz/mpatch_corpus.py +++ b/contrib/fuzz/mpatch_corpus.py @@ -78,6 +78,10 @@ with zipfile.ZipFile(args.out[0], "w", z zf.writestr( "mpatch_decode_old_overread", "\x02\x00\x00\x00\x02\x00\x00\x00" ) + # https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=8876 + zf.writestr( + "mpatch_ossfuzz_getbe32_ubsan", + "\x02\x00\x00\x00\x0c \xff\xff\xff\xff ") zf.writestr( "mpatch_apply_over_memcpy", '\x13\x01\x00\x05\xd0\x00\x00\x00\x00\x00\x00\x00\x00\n \x00\x00\x00' diff --git a/mercurial/bitmanipulation.h b/mercurial/bitmanipulation.h --- a/mercurial/bitmanipulation.h +++ b/mercurial/bitmanipulation.h @@ -9,7 +9,8 @@ static inline uint32_t getbe32(const cha { const unsigned char *d = (const unsigned char *)c; - return ((d[0] << 24) | (d[1] << 16) | (d[2] << 8) | (d[3])); + return ((((uint32_t)d[0]) << 24) | (((uint32_t)d[1]) << 16) | + (((uint32_t)d[2]) << 8) | (d[3])); } static inline int16_t getbeint16(const char *c)