From 81c2953ccd5342c33fff1e3a7264e36a483808b8 Mon Sep 17 00:00:00 2001 From: "Gabriel A. Devenyi" Date: Fri, 14 Aug 2026 23:14:27 -0400 Subject: [PATCH] BUG: Use memcpy instead of casting to over-aligned pointer types 19 -Wcast-align warnings, and behind them undefined behavior on any target that cares about alignment. modify_field() writes a value into a header field at a byte offset parsed from a field table: ((short *)((char *)basep + field->offset))[fc] = (short)val; field->offset is a byte offset into a packed on-disk header, so that cast produces an address only correctly aligned by coincidence. The same pattern appears for int, int64_t, float and double, in both tool files. Each becomes a memcpy of the right width at the right byte offset. The second group reads a pointer back out of a structure through a byte offset -- `sp = *(char **)((char *)str + fp->offset)` and the nifti1_extension equivalents -- and becomes a memcpy into an aligned local. The third is nifti_header_version(), which cast its `const char * buf` argument, a buffer straight off a file read with no alignment guarantee, to both nifti_1_header * and nifti_2_header * and read fields through them. It now copies into aligned locals first, exactly the sizeof(nifti_1_header) bytes the function already checks are present. Verified by round-tripping int16, int32, int64, float32, float64 and string fields through nifti_tool -mod_hdr2. --- nifti2/nifti2_io.c | 29 +++++++++++++++++++---------- nifti2/nifti_tool.c | 24 ++++++++++++++---------- niftilib/nifti1_tool.c | 20 ++++++++++++-------- 3 files changed, 45 insertions(+), 28 deletions(-) diff --git a/nifti2/nifti2_io.c b/nifti2/nifti2_io.c index 58bc2bfb..c97611f9 100644 --- a/nifti2/nifti2_io.c +++ b/nifti2/nifti2_io.c @@ -6614,8 +6614,8 @@ int valid_nifti_extensions(const nifti_image * nim) \return -1 on error, else NIFTI version *//*--------------------------------------------------------------------*/ int nifti_header_version(const char * buf, size_t nbytes){ - const nifti_1_header *n1p = (const nifti_1_header *)buf; - const nifti_2_header *n2p = (const nifti_2_header *)buf; + nifti_1_header n1hdr; + nifti_2_header n2hdr; char fname[] = { "nifti_header_version" }; int sizeof_hdr, sver, nver; @@ -6631,9 +6631,18 @@ int nifti_header_version(const char * buf, size_t nbytes){ return -1; } + /* buf comes straight from a file read and need not satisfy the alignment + either header struct requires, so work from aligned copies rather than + casting it. Only sizeof(nifti_1_header) bytes are guaranteed present, + and both sizeof_hdr and magic fall inside that range for either + version, so copy exactly that much into each. */ + memcpy(&n1hdr, buf, sizeof(n1hdr)); + memset(&n2hdr, 0, sizeof(n2hdr)); + memcpy(&n2hdr, buf, sizeof(n1hdr)); + /* try to determine the version based on sizeof_hdr */ sver = -1; - sizeof_hdr = n1p->sizeof_hdr; + sizeof_hdr = n1hdr.sizeof_hdr; if ( sizeof_hdr == (int)sizeof(nifti_1_header) ) sver = 1; else if( sizeof_hdr == (int)sizeof(nifti_2_header) ) sver = 2; else { /* try swapping */ @@ -6643,8 +6652,8 @@ int nifti_header_version(const char * buf, size_t nbytes){ } /* and check magic field */ - if ( sver == 1 ) nver = NIFTI_VERSION(*n1p); - else if ( sver == 2 ) nver = NIFTI_VERSION(*n2p); + if ( sver == 1 ) nver = NIFTI_VERSION(n1hdr); + else if ( sver == 2 ) nver = NIFTI_VERSION(n2hdr); else nver = -1; /* now compare and return */ @@ -6653,24 +6662,24 @@ int nifti_header_version(const char * buf, size_t nbytes){ fprintf(stderr,"-- %s: size ver = %d, ni ver = %d\n", fname, sver, nver); if( sver == 1 ) { - nver = NIFTI_VERSION(*n1p); + nver = NIFTI_VERSION(n1hdr); if( nver == 0 ) return 0; /* ANALYZE */ if( nver == 1 ) return 1; /* NIFTI-1 */ if( g_opts.debug > 1 ) - fprintf(stderr,"** %s: bad NIFTI-1 magic= %.4s", fname, n1p->magic); + fprintf(stderr,"** %s: bad NIFTI-1 magic= %.4s", fname, n1hdr.magic); return -1; } else if ( sver == 2 ) { - nver = NIFTI_VERSION(*n2p); + nver = NIFTI_VERSION(n2hdr); if( nver == 2 ) return 2; /* NIFTI-2 */ if( g_opts.debug > 1 ) - fprintf(stderr,"** %s: bad NIFTI-2 magic4= %.4s", fname, n2p->magic); + fprintf(stderr,"** %s: bad NIFTI-2 magic4= %.4s", fname, n2hdr.magic); return -1; } /* failure */ if( g_opts.debug > 0 ) - fprintf(stderr,"** %s: bad sizeof_hdr = %d\n", fname, n1p->sizeof_hdr); + fprintf(stderr,"** %s: bad sizeof_hdr = %d\n", fname, n1hdr.sizeof_hdr); return -1; } diff --git a/nifti2/nifti_tool.c b/nifti2/nifti_tool.c index 85bc3f54..d2d59a9e 100644 --- a/nifti2/nifti_tool.c +++ b/nifti2/nifti_tool.c @@ -3928,7 +3928,8 @@ int modify_field(void * basep, field_s * field, const char * data) return 1; } /* otherwise, we're good */ - ((short *)((char *)basep + field->offset))[fc] = (short)val; + { const int16_t sval = (int16_t)val; + memcpy((char *)basep + field->offset + (size_t)fc * sizeof(sval), &sval,(size_t)sizeof(sval)); } if( g_debug > 1 ) fprintf(stderr,"+d setting posn %d of '%s' to %d\n", fc, field->name, val); @@ -3947,7 +3948,8 @@ int modify_field(void * basep, field_s * field, const char * data) fc,field->len); return 1; } - ((int *)((char *)basep + field->offset))[fc] = val; + { const int32_t ival = (int32_t)val; + memcpy((char *)basep + field->offset + (size_t)fc * sizeof(ival), &ival,(size_t)sizeof(ival)); } if( g_debug > 1 ) fprintf(stderr,"+d setting posn %d of '%s' to %d\n", fc, field->name, val); @@ -3967,7 +3969,7 @@ int modify_field(void * basep, field_s * field, const char * data) fc,field->len); return 1; } - ((int64_t *)((char *)basep + field->offset))[fc] = v64; + memcpy((char *)basep + field->offset + (size_t)fc * sizeof(v64), &v64,(size_t)sizeof(v64)); if( g_debug > 1 ) fprintf(stderr,"+d setting posn %d of '%s' to %" PRId64 "\n", fc, field->name, v64); @@ -3987,7 +3989,7 @@ int modify_field(void * basep, field_s * field, const char * data) return 1; } /* otherwise, we're good */ - ((float *)((char *)basep + field->offset))[fc] = fval; + memcpy((char *)basep + field->offset + (size_t)fc * sizeof(fval), &fval,(size_t)sizeof(fval)); if( g_debug > 1 ) fprintf(stderr,"+d setting posn %d of '%s' to %f\n", fc, field->name, fval); @@ -4008,7 +4010,7 @@ int modify_field(void * basep, field_s * field, const char * data) return 1; } /* otherwise, we're good */ - ((double *)((char *)basep + field->offset))[fc] = f64; + memcpy((char *)basep + field->offset + (size_t)fc * sizeof(f64), &f64,(size_t)sizeof(f64)); if( g_debug > 1 ) fprintf(stderr,"+d setting posn %d of '%s' to %f\n", fc, field->name, f64); @@ -6271,7 +6273,7 @@ int disp_field(const char *mesg, field_s *fieldp, void * str, int nfields, int h int len; /* start by sucking the pointer stored here */ - sp = *(char **)((char *)str + fp->offset); + memcpy(&sp, (const char *)str + fp->offset, sizeof(sp)); if( ! sp ){ fprintf(stdout,"(NULL)\n"); break; } /* anything? */ @@ -6285,7 +6287,9 @@ int disp_field(const char *mesg, field_s *fieldp, void * str, int nfields, int h else if( *sp && !isprint(*sp) ) /* if no termination, it's bad */ fprintf(stdout,"(non-printable string)\n"); else /* woohoo! a good string */ - fprintf(stdout,"'%.40s'\n",*(char **)((char *)str + fp->offset)); + { char * cp; + memcpy(&cp, (const char *)str + fp->offset, sizeof(cp)); + fprintf(stdout,"'%.40s'\n", cp); } break; } @@ -6294,7 +6298,7 @@ int disp_field(const char *mesg, field_s *fieldp, void * str, int nfields, int h nifti1_extension * extp; /* yank the address sitting there into extp */ - extp = *(nifti1_extension **)((char *)str + fp->offset); + memcpy(&extp, (const char *)str + fp->offset, sizeof(extp)); /* the user may use -disp_exts to display all of them */ if( extp ) disp_nifti1_extension(NULL, extp, 6); @@ -6355,8 +6359,8 @@ int diff_field(field_s *fieldp, void * str0, void * str1, int nfields) { nifti1_extension * ext0, * ext1; - ext0 = *(nifti1_extension **)((char *)str0 + fp->offset); - ext1 = *(nifti1_extension **)((char *)str1 + fp->offset); + memcpy(&ext0, (const char *)str0 + fp->offset, sizeof(ext0)); + memcpy(&ext1, (const char *)str1 + fp->offset, sizeof(ext1)); if( ! ext0 && ! ext1 ) break; /* continue on */ diff --git a/niftilib/nifti1_tool.c b/niftilib/nifti1_tool.c index 7bd422b1..d4982643 100644 --- a/niftilib/nifti1_tool.c +++ b/niftilib/nifti1_tool.c @@ -2980,7 +2980,8 @@ int modify_field(void * basep, field_s * field, const char * data) return 1; } /* otherwise, we're good */ - ((short *)((char *)basep + field->offset))[fc] = (short)val; + { const int16_t sval = (int16_t)val; + memcpy((char *)basep + field->offset + (size_t)fc * sizeof(sval), &sval,(size_t)sizeof(sval)); } if( g_debug > 1 ) fprintf(stderr,"+d setting posn %d of '%s' to %d\n", fc, field->name, val); @@ -2999,7 +3000,8 @@ int modify_field(void * basep, field_s * field, const char * data) fc,field->len); return 1; } - ((int *)((char *)basep + field->offset))[fc] = val; + { const int32_t ival = (int32_t)val; + memcpy((char *)basep + field->offset + (size_t)fc * sizeof(ival), &ival,(size_t)sizeof(ival)); } if( g_debug > 1 ) fprintf(stderr,"+d setting posn %d of '%s' to %d\n", fc, field->name, val); @@ -3019,7 +3021,7 @@ int modify_field(void * basep, field_s * field, const char * data) return 1; } /* otherwise, we're good */ - ((float *)((char *)basep + field->offset))[fc] = fval; + memcpy((char *)basep + field->offset + (size_t)fc * sizeof(fval), &fval,(size_t)sizeof(fval)); if( g_debug > 1 ) fprintf(stderr,"+d setting posn %d of '%s' to %f\n", fc, field->name, fval); @@ -3478,7 +3480,7 @@ int disp_field( const char *mesg, field_s *fieldp, void * str, int nfields, int int len; /* start by sucking the pointer stored here */ - sp = *(char **)((char *)str + fp->offset); + memcpy(&sp, (const char *)str + fp->offset, sizeof(sp)); if( ! sp ){ fprintf(stdout,"(NULL)\n"); break; } /* anything? */ @@ -3492,7 +3494,9 @@ int disp_field( const char *mesg, field_s *fieldp, void * str, int nfields, int else if( *sp && !isprint(*sp) ) /* if no termination, it's bad */ fprintf(stdout,"(non-printable string)\n"); else /* woohoo! a good string */ - fprintf(stdout,"'%.40s'\n",*(char **)((char *)str + fp->offset)); + { char * cp; + memcpy(&cp, (const char *)str + fp->offset, sizeof(cp)); + fprintf(stdout,"'%.40s'\n", cp); } break; } @@ -3501,7 +3505,7 @@ int disp_field( const char *mesg, field_s *fieldp, void * str, int nfields, int nifti1_extension * extp; /* yank the address sitting there into extp */ - extp = *(nifti1_extension **)((char *)str + fp->offset); + memcpy(&extp, (const char *)str + fp->offset, sizeof(extp)); /* the user may use -disp_exts to display all of them */ if( extp ) disp_nifti1_extension(NULL, extp, 6); @@ -3560,8 +3564,8 @@ int diff_field(field_s *fieldp, void * str0, void * str1, int nfields) { nifti1_extension * ext0, * ext1; - ext0 = *(nifti1_extension **)((char *)str0 + fp->offset); - ext1 = *(nifti1_extension **)((char *)str1 + fp->offset); + memcpy(&ext0, (const char *)str0 + fp->offset, sizeof(ext0)); + memcpy(&ext1, (const char *)str1 + fp->offset, sizeof(ext1)); if( ! ext0 && ! ext1 ) break; /* continue on */