[File] [PATCH] Prevent unaligned reads of 'union VALUETYPE' in mget

Christos Zoulas christos at zoulas.com
Fri Jul 19 18:50:03 UTC 2024


Fixed, thanks!

christos

> On Jul 17, 2024, at 1:19 AM, Anthony Steinhauser <asteinhauser at google.com> wrote:
> 
> When compiled with ASAN, libmagic currently crashes with:
> 
> softmagic.c:1674:1: runtime error: member access within misaligned address
> 0x7fccc9cff174 for type 'const union VALUETYPE', which requires 8 byte
> alignment
> 0x7fccc9cff174: note: pointer points here
>  00 30 00 00 e0 08 00 00  00 18 00 00 00 00 00 00  00 00 00 00 00 00 00 00
> 40 00 00 40 2e 53 61 64
>              ^
> softmagic.c:1677:11: runtime error: member access within misaligned address
> 0x7fccc9cff174 for type 'const union VALUETYPE', which requires 8 byte
> alignment
> 0x7fccc9cff174: note: pointer points here
>  00 30 00 00 e0 08 00 00  00 18 00 00 00 00 00 00  00 00 00 00 00 00 00 00
> 40 00 00 40 2e 53 61 64
>              ^
> softmagic.c:1677:11: runtime error: member access within misaligned address
> 0x7fccc9cff174 for type 'const union VALUETYPE', which requires 8 byte
> alignment
> 0x7fccc9cff174: note: pointer points here
>  00 30 00 00 e0 08 00 00  00 18 00 00 00 00 00 00  00 00 00 00 00 00 00 00
> 40 00 00 40 2e 53 61 64
>              ^
> softmagic.c:1677:11: runtime error: member access within misaligned address
> 0x7fccc9cff174 for type 'const union VALUETYPE', which requires 8 byte
> alignment
> 0x7fccc9cff174: note: pointer points here
>  00 30 00 00 e0 08 00 00  00 18 00 00 00 00 00 00  00 00 00 00 00 00 00 00
> 40 00 00 40 2e 53 61 64
>              ^
> softmagic.c:1677:11: runtime error: member access within misaligned address
> 0x7fccc9cff174 for type 'const union VALUETYPE', which requires 8 byte
> alignment
> 0x7fccc9cff174: note: pointer points here
>  00 30 00 00 e0 08 00 00  00 18 00 00 00 00 00 00  00 00 00 00 00 00 00 00
> 40 00 00 40 2e 53 61 64
> 
> when processing some PE32 files. The same problem was reported already in:
> https://bugs.astron.com/view.php?id=486
> 
> However I encountered it in production code, not only by fuzzing.
> 
> ---
> src/file.h      |  3 ---
> src/softmagic.c | 60 ++++++++++++++++++++++++-------------------------
> 2 files changed, 30 insertions(+), 33 deletions(-)
> 
> diff --git a/src/file.h b/src/file.h
> index 1f2ff004..33200e03 100644
> --- a/src/file.h
> +++ b/src/file.h
> @@ -206,9 +206,6 @@ union VALUETYPE {
> 	uint16_t h;
> 	uint32_t l;
> 	uint64_t q;
> -	uint8_t hs[2];	/* 2 bytes of a fixed-endian "short" */
> -	uint8_t hl[4];	/* 4 bytes of a fixed-endian "long" */
> -	uint8_t hq[8];	/* 8 bytes of a fixed-endian "quad" */
> 	char s[MAXstring];	/* the search string or regex pattern */
> 	unsigned char us[MAXstring];
> 	uint64_t guid[2];
> diff --git a/src/softmagic.c b/src/softmagic.c
> index ea466ecd..84b4d92a 100644
> --- a/src/softmagic.c
> +++ b/src/softmagic.c
> @@ -71,41 +71,41 @@ file_private int cvt_64(union VALUETYPE *, const struct magic *);
> 
> #define OFFSET_OOB(n, o, i)	((n) < CAST(uint32_t, (o)) || (i) > ((n) - (o)))
> #define BE64(p) ( \
> -    (CAST(uint64_t, (p)->hq[0])<<56)| \
> -    (CAST(uint64_t, (p)->hq[1])<<48)| \
> -    (CAST(uint64_t, (p)->hq[2])<<40)| \
> -    (CAST(uint64_t, (p)->hq[3])<<32)| \
> -    (CAST(uint64_t, (p)->hq[4])<<24)| \
> -    (CAST(uint64_t, (p)->hq[5])<<16)| \
> -    (CAST(uint64_t, (p)->hq[6])<<8)| \
> -    (CAST(uint64_t, (p)->hq[7])))
> +    (CAST(uint64_t, *CAST(const uint8_t *, p))<<56)| \
> +    (CAST(uint64_t, *(CAST(const uint8_t *, p)+1))<<48)| \
> +    (CAST(uint64_t, *(CAST(const uint8_t *, p)+2))<<40)| \
> +    (CAST(uint64_t, *(CAST(const uint8_t *, p)+3))<<32)| \
> +    (CAST(uint64_t, *(CAST(const uint8_t *, p)+4))<<24)| \
> +    (CAST(uint64_t, *(CAST(const uint8_t *, p)+5))<<16)| \
> +    (CAST(uint64_t, *(CAST(const uint8_t *, p)+6))<<8)| \
> +    (CAST(uint64_t, *(CAST(const uint8_t *, p)+7))))
> #define LE64(p) ( \
> -    (CAST(uint64_t, (p)->hq[7])<<56)| \
> -    (CAST(uint64_t, (p)->hq[6])<<48)| \
> -    (CAST(uint64_t, (p)->hq[5])<<40)| \
> -    (CAST(uint64_t, (p)->hq[4])<<32)| \
> -    (CAST(uint64_t, (p)->hq[3])<<24)| \
> -    (CAST(uint64_t, (p)->hq[2])<<16)| \
> -    (CAST(uint64_t, (p)->hq[1])<<8)| \
> -    (CAST(uint64_t, (p)->hq[0])))
> +    (CAST(uint64_t, *(CAST(const uint8_t *, p)+7))<<56)| \
> +    (CAST(uint64_t, *(CAST(const uint8_t *, p)+6))<<48)| \
> +    (CAST(uint64_t, *(CAST(const uint8_t *, p)+5))<<40)| \
> +    (CAST(uint64_t, *(CAST(const uint8_t *, p)+4))<<32)| \
> +    (CAST(uint64_t, *(CAST(const uint8_t *, p)+3))<<24)| \
> +    (CAST(uint64_t, *(CAST(const uint8_t *, p)+2))<<16)| \
> +    (CAST(uint64_t, *(CAST(const uint8_t *, p)+1))<<8)| \
> +    (CAST(uint64_t, *CAST(const uint8_t *, p))))
> #define LE32(p) ( \
> -    (CAST(uint32_t, (p)->hl[3])<<24)| \
> -    (CAST(uint32_t, (p)->hl[2])<<16)| \
> -    (CAST(uint32_t, (p)->hl[1])<<8)| \
> -    (CAST(uint32_t, (p)->hl[0])))
> +    (CAST(uint32_t, *(CAST(const uint8_t *, p)+3))<<24)| \
> +    (CAST(uint32_t, *(CAST(const uint8_t *, p)+2))<<16)| \
> +    (CAST(uint32_t, *(CAST(const uint8_t *, p)+1))<<8)| \
> +    (CAST(uint32_t, *CAST(const uint8_t *, p))))
> #define BE32(p) ( \
> -    (CAST(uint32_t, (p)->hl[0])<<24)| \
> -    (CAST(uint32_t, (p)->hl[1])<<16)| \
> -    (CAST(uint32_t, (p)->hl[2])<<8)| \
> -    (CAST(uint32_t, (p)->hl[3])))
> +    (CAST(uint32_t, *CAST(const uint8_t *, p))<<24)| \
> +    (CAST(uint32_t, *(CAST(const uint8_t *, p)+1))<<16)| \
> +    (CAST(uint32_t, *(CAST(const uint8_t *, p)+2))<<8)| \
> +    (CAST(uint32_t, *(CAST(const uint8_t *, p)+3))))
> #define ME32(p) ( \
> -    (CAST(uint32_t, (p)->hl[1])<<24)| \
> -    (CAST(uint32_t, (p)->hl[0])<<16)| \
> -    (CAST(uint32_t, (p)->hl[3])<<8)| \
> -    (CAST(uint32_t, (p)->hl[2])))
> +    (CAST(uint32_t, *(CAST(const uint8_t *, p)+1))<<24)| \
> +    (CAST(uint32_t, *CAST(const uint8_t *, p))<<16)| \
> +    (CAST(uint32_t, *(CAST(const uint8_t *, p)+3))<<8)| \
> +    (CAST(uint32_t, *(CAST(const uint8_t *, p)+2))))
> 
> -#define BE16(p) ((CAST(uint16_t, (p)->hs[0])<<8)|(CAST(uint16_t, (p)->hs[1])))
> -#define LE16(p) ((CAST(uint16_t, (p)->hs[1])<<8)|(CAST(uint16_t, (p)->hs[0])))
> +#define BE16(p) (CAST(uint16_t, *CAST(const uint8_t *, p))<<8)|(CAST(uint16_t, *(CAST(const uint8_t *, p)+1)))
> +#define LE16(p) (CAST(uint16_t, *(CAST(const uint8_t *, p)+1))<<8)|(CAST(uint16_t, *CAST(const uint8_t *, p)))
> #define SEXT(s,v,p) ((s) ? \
> 	CAST(intmax_t, CAST(int##v##_t, p)) : \
> 	CAST(intmax_t, CAST(uint##v##_t, p)))
> -- 
> 2.45.2.993.g49e7a77208-goog



More information about the File mailing list