Skip to content

BUG: Use memcpy instead of casting to over-aligned pointer types #44

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hjmjohnson merged 1 commit into from
Sep 22, 2026
Merged

BUG: Use memcpy instead of casting to over-aligned pointer types #44

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 19 additions & 10 deletions nifti2/nifti2_io.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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 */
Expand All @@ -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 */
Expand All @@ -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;
}
Expand Down
24 changes: 14 additions & 10 deletions nifti2/nifti_tool.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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? */

Expand All @@ -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;
}

Expand All @@ -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);
Expand Down Expand Up @@ -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 */

Expand Down
20 changes: 12 additions & 8 deletions niftilib/nifti1_tool.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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? */

Expand All @@ -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;
}

Expand All @@ -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);
Expand Down Expand Up @@ -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 */

Expand Down
Loading