( )⚙ D3729 bitmanipulation: fix undefined behavior in bit shift in getbe32

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D3729

bitmanipulation: fix undefined behavior in bit shift in getbe32
ClosedPublic

Authored by durin42 on Jun 13 2018, 10:24 AM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG1fb2510cf8c8: bitmanipulation: fix undefined behavior in bit shift in getbe32
Summary

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

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

durin42 created this revision.Jun 13 2018, 10:24 AM
Closed by commit rHG1fb2510cf8c8: bitmanipulation: fix undefined behavior in bit shift in getbe32 (authored by durin42). · Explain WhyJun 13 2018, 1:31 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 9052

contrib/fuzz/mpatch_corpus.py

zf.writestr('manifest_py_rev_10', zf.writestr('manifest_py_rev_10',
str(corpus(bins[0], bins[1:]))) str(corpus(bins[0], bins[1:])))
except: # skip this, so no re-raises except: # skip this, so no re-raises
print('skipping seed file from repo data') print('skipping seed file from repo data')
# Automatically discovered by running the fuzzer # Automatically discovered by running the fuzzer
zf.writestr( zf.writestr(
"mpatch_decode_old_overread", "\x02\x00\x00\x00\x02\x00\x00\x00" "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( zf.writestr(
"mpatch_apply_over_memcpy", "mpatch_apply_over_memcpy",
'\x13\x01\x00\x05\xd0\x00\x00\x00\x00\x00\x00\x00\x00\n \x00\x00\x00' '\x13\x01\x00\x05\xd0\x00\x00\x00\x00\x00\x00\x00\x00\n \x00\x00\x00'
'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00' '\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'
'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xff\xff\x00\x00\x00\x00\x00' '\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xff\xff\x00\x00\x00\x00\x00'
'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00' '\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'
'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00' '\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'
'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00' '\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'

mercurial/bitmanipulation.h

#ifndef _HG_BITMANIPULATION_H_ #ifndef _HG_BITMANIPULATION_H_
#define _HG_BITMANIPULATION_H_ #define _HG_BITMANIPULATION_H_
#include <string.h> #include <string.h>
#include "compat.h" #include "compat.h"
static inline uint32_t getbe32(const char *c) static inline uint32_t getbe32(const char *c)
{ {
const unsigned char *d = (const unsigned char *)c; 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) static inline int16_t getbeint16(const char *c)
{ {
const unsigned char *d = (const unsigned char *)c; const unsigned char *d = (const unsigned char *)c;
return ((d[0] << 8) | (d[1])); return ((d[0] << 8) | (d[1]));
} }