Commit 99d821db4 for imagemagick.org
commit 99d821db45ed03afc80925ce11eff62abe80484a
Author: Cristy <urban-warrior@imagemagick.org>
Date: Sun Jul 5 15:57:00 2026 -0400
address remote TOCTOU issues
diff --git a/MagickCore/policy-private.h b/MagickCore/policy-private.h
index 373b5f8da..839e6acdc 100644
--- a/MagickCore/policy-private.h
+++ b/MagickCore/policy-private.h
@@ -41,13 +41,105 @@ extern MagickPrivate MagickBooleanType
extern MagickPrivate void
PolicyComponentTerminus(void);
+static inline MagickBooleanType IsPathContainsSymlink(const char *path)
+{
+ char
+ partial[MagickPathExtent];
+
+ const char
+ *p;
+
+ ssize_t
+ offset = 0;
+
+ if (path == (const char *) NULL)
+ return(MagickFalse);
+ *partial='\0';
+ p=path;
+ if (*p == *DirectorySeparator)
+ {
+ /*
+ Path starts with a directory separator, include it.
+ */
+ if ((offset+1) >= (ssize_t) sizeof(partial))
+ return(MagickFalse);
+ partial[offset++]=(*p++);
+ partial[offset]='\0';
+ }
+ while (*p != '\0')
+ {
+ char
+ component[MagickPathExtent];
+
+ ssize_t
+ i = 0;
+
+ /*
+ Copy next component into a temporary buffer.
+ */
+ while ((*p != '\0') && (*p != *DirectorySeparator) &&
+ ((i+1) < (ssize_t) sizeof(component)))
+ component[i++]=(*p++);
+ component[i]='\0';
+ if (i == 0)
+ {
+ /*
+ skip repeated separators.
+ */
+ if (*p == *DirectorySeparator)
+ p++;
+ continue;
+ }
+ if ((offset > 0) && (partial[offset-1] != *DirectorySeparator))
+ {
+ /*
+ Append separator if needed.
+ */
+ if ((offset+1) >= (ssize_t) sizeof(partial))
+ return MagickFalse;
+ partial[offset++]=(*DirectorySeparator);
+ partial[offset]='\0';
+ }
+ /*
+ Append component.
+ */
+ if ((offset+i) >= (ssize_t) sizeof(partial))
+ return(MagickFalse);
+ (void) memcpy(partial+offset,component,i);
+ offset+=i;
+ partial[offset]='\0';
+ if (*p != '\0')
+ {
+ /*
+ Check whether this prefix is a symlink.
+ */
+ if (is_symlink_utf8(partial) != MagickFalse)
+ return(MagickTrue);
+ }
+ /*
+ Skip separator.
+ */
+ if (*p == *DirectorySeparator)
+ p++;
+ }
+ return(MagickFalse);
+}
+
static inline MagickBooleanType IsPathAuthorized(const PolicyRights rights,
const char *filename)
{
+ MagickBooleanType symlink_follow_allowed = IsRightsAuthorizedByName(
+ SystemPolicyDomain,"symlink",rights,"follow");
MagickBooleanType status =
((IsRightsAuthorized(PathPolicyDomain,rights,filename) != MagickFalse) &&
- ((IsRightsAuthorizedByName(SystemPolicyDomain,"symlink",rights,"follow") != MagickFalse) ||
+ ((symlink_follow_allowed != MagickFalse) ||
(is_symlink_utf8(filename) == MagickFalse))) ? MagickTrue : MagickFalse;
+ if ((status != MagickFalse) && (symlink_follow_allowed == MagickFalse))
+ {
+ if ((is_symlink_utf8(filename) != MagickFalse) ||
+ (IsPathContainsSymlink(filename) != MagickFalse))
+ status=MagickFalse;
+ }
return(status);
}
diff --git a/MagickCore/policy.c b/MagickCore/policy.c
index 585557923..282b0539c 100644
--- a/MagickCore/policy.c
+++ b/MagickCore/policy.c
@@ -661,12 +661,14 @@ MagickExport MagickBooleanType IsRightsAuthorizedByName(
*exception;
MagickBooleanType
+ canonical_matched_any = MagickFalse,
matched_any = MagickFalse,
paths_provisioned = MagickFalse,
status;
PolicyRights
- effective_rights = AllPolicyRights; /* rights authorized unless denied */
+ canonical_allowed_accumulator = AllPolicyRights,
+ effective_rights = AllPolicyRights;
/*
Load policies.
@@ -689,7 +691,8 @@ MagickExport MagickBooleanType IsRightsAuthorizedByName(
return(MagickTrue);
}
/*
- Evaluate policies in order; last match wins.
+ Evaluate policies in order; last match wins, however, canonical denies are
+ enforced after evaluation.
*/
LockSemaphoreInfo(policy_semaphore);
ResetLinkedListIterator(policy_cache);
@@ -700,7 +703,8 @@ MagickExport MagickBooleanType IsRightsAuthorizedByName(
*policy = (PolicyInfo *) p->value;
MagickBooleanType
- match = MagickFalse;
+ match = MagickFalse,
+ matched_canonical = MagickFalse;
if (policy->domain != domain)
continue;
@@ -741,11 +745,30 @@ MagickExport MagickBooleanType IsRightsAuthorizedByName(
match=GlobExpression(canonical_candidate,policy->pattern,MagickFalse);
if ((canonical_path != (char *) NULL) && (match == MagickFalse))
match=GlobExpression(canonical_path,policy->pattern,MagickFalse);
+ if ((canonical_path != (char *) NULL) &&
+ (GlobExpression(canonical_path,policy->pattern,MagickFalse) != MagickFalse))
+ matched_canonical=MagickTrue;
+ else
+ if ((canonical_candidate != (char *) NULL) &&
+ (GlobExpression(canonical_candidate,policy->pattern,MagickFalse) != MagickFalse))
+ matched_canonical=MagickTrue;
+ else
+ if ((canonical_directory != (char *) NULL) &&
+ (GlobExpression(canonical_directory,policy->pattern,MagickFalse) != MagickFalse))
+ matched_canonical=MagickTrue;
}
if (match == MagickFalse)
continue;
matched_any=MagickTrue;
effective_rights=policy->rights;
+ if (matched_canonical != MagickFalse)
+ {
+ /*
+ If this match was against a canonical form, accumulate allowed rights.
+ */
+ canonical_matched_any=MagickTrue;
+ canonical_allowed_accumulator&=policy->rights;
+ }
}
UnlockSemaphoreInfo(policy_semaphore);
if (canonical_directory != (char *) NULL)
@@ -770,6 +793,16 @@ MagickExport MagickBooleanType IsRightsAuthorizedByName(
((effective_rights & ExecutePolicyRights) == 0))
status=MagickFalse;
}
+ /*
+ Enforce sticky canonical denies.
+ */
+ if (canonical_matched_any != MagickFalse)
+ {
+ PolicyRights canonical_denied_mask = (AllPolicyRights &
+ ~canonical_allowed_accumulator);
+ if ((canonical_denied_mask & rights) != 0)
+ status=MagickFalse;
+ }
if ((GetLogEventMask() & PolicyEvent) != 0)
(void) LogMagickEvent(PolicyEvent,GetMagickModule(),
" authorized: %s",status == MagickFalse ? "false" : "true");
diff --git a/MagickCore/utility-private.h b/MagickCore/utility-private.h
index d644c2c14..dacf64c1a 100644
--- a/MagickCore/utility-private.h
+++ b/MagickCore/utility-private.h
@@ -47,6 +47,49 @@ extern MagickPrivate void
ChopPathComponents(char *,const size_t),
ExpandFilename(char *);
+static inline MagickBooleanType GetPathSignature(int file,char signature[33])
+{
+ char
+ *p = signature;
+
+ ssize_t
+ i;
+
+ static const char
+ hex[] = "0123456789abcdef";
+
+ uint64_t
+ file_identity[2];
+
+#if defined(MAGICKCORE_WINDOWS_SUPPORT)
+ BY_HANDLE_FILE_INFORMATION file_info;
+ HANDLE handle = (HANDLE) _get_osfhandle(file);
+ if (handle == INVALID_HANDLE_VALUE)
+ return(MagickFalse);
+ if (GetFileInformationByHandle(handle,&file_info) == 0)
+ return(MagickFalse);
+ file_identity[0]=(uint64_t) file_info.dwVolumeSerialNumber;
+ file_identity[1]=((uint64_t) file_info.nFileIndexHigh << 32) |
+ (uint64_t) file_info.nFileIndexLow;
+#else
+ struct stat file_attributes;
+ if (fstat(file, &file_attributes) != 0)
+ return(MagickFalse);
+ file_identity[0]=(uint64_t) file_attributes.st_dev;
+ file_identity[1]=(uint64_t) file_attributes.st_ino;
+#endif
+ for (i=0; i < 2; i++)
+ {
+ ssize_t
+ shift;
+
+ for (shift=60; shift >= 0; shift-=4)
+ *p++=hex[(file_identity[i] >> shift) & 0xF];
+ }
+ *p='\0';
+ return(MagickTrue);
+}
+
static inline int MagickReadDirectory(DIR *directory,struct dirent *entry,
struct dirent **result)
{
diff --git a/MagickWand/compare.c b/MagickWand/compare.c
index 772a38325..f5483e218 100644
--- a/MagickWand/compare.c
+++ b/MagickWand/compare.c
@@ -1244,10 +1244,6 @@ WandExport MagickBooleanType CompareImagesCommand(ImageInfo *image_info,
switch (metric)
{
case AbsoluteErrorMetric:
- {
- scale=1.0;
- break;
- }
case PixelDifferenceCountErrorMetric:
{
size_t
@@ -1330,10 +1326,9 @@ WandExport MagickBooleanType CompareImagesCommand(ImageInfo *image_info,
switch (metric)
{
case AbsoluteErrorMetric:
- case PixelDifferenceCountErrorMetric:
{
- (void) FormatLocaleFile(stderr,"%.*g (%.*g)",GetMagickPrecision(),
- (scale*distortion),GetMagickPrecision(),distortion);
+ (void) FormatLocaleFile(stderr,"%.20g (%.20g)",distortion,
+ distortion/scale);
break;
}
case MeanErrorPerPixelErrorMetric:
diff --git a/coders/mat.c b/coders/mat.c
index e5eb6982f..6cc658af8 100644
--- a/coders/mat.c
+++ b/coders/mat.c
@@ -519,7 +519,7 @@ ssize_t total_size = 0;
mat_file=0;
file = AcquireUniqueFileResource(clone_info->filename);
if (file != -1)
- mat_file = fdopen(file,"w");
+ mat_file = fdopen(file,"ab+");
if(!mat_file)
{
RelinquishMagickMemory(cache_block);
@@ -576,12 +576,14 @@ ssize_t total_size = 0;
DblBreak:
inflateEnd(&zip_info);
- (void)fclose(mat_file);
+ if (fseek(mat_file,0,SEEK_SET) != 0)
+ (void) ThrowMagickException(exception,GetMagickModule(),CorruptImageError,
+ "UnableToCreateTemporaryFile","`%s'",clone_info->filename);
RelinquishMagickMemory(cache_block);
RelinquishMagickMemory(decompress_block);
*Size = (unsigned int) total_size;
- if((clone_info->file=fopen_utf8(clone_info->filename,"rb"))==NULL) goto UnlinkFile;
+ clone_info->file=mat_file;
if( (image2 = AcquireImage(clone_info,exception))==NULL ) goto EraseFile;
image2->columns=0;
image2->rows=0;
@@ -592,7 +594,6 @@ DblBreak:
EraseFile:
fclose(clone_info->file);
clone_info->file = NULL;
-UnlinkFile:
RelinquishUniqueFileResource(clone_info->filename);
return NULL;
}
diff --git a/coders/ora.c b/coders/ora.c
index 7a2019064..50c6dd001 100644
--- a/coders/ora.c
+++ b/coders/ora.c
@@ -174,7 +174,7 @@ static Image *ReadORAImage(const ImageInfo *image_info,
MagickPathExtent);
file=(FILE *) NULL;
if (unique_file != -1)
- file=fdopen(unique_file,"wb");
+ file=fdopen(unique_file,"rb+");
if ((unique_file == -1) || (file == (FILE *) NULL))
{
ThrowFileException(exception,FileOpenError,"UnableToCreateTemporaryFile",
@@ -214,19 +214,22 @@ static Image *ReadORAImage(const ImageInfo *image_info,
else
offset+=(zip_uint64_t) read_bytes;
}
- (void) fclose(file);
+ if (fseek(file,0,SEEK_SET) != 0)
+ status=MagickFalse;
(void) zip_fclose(merged_image_file);
(void) zip_discard(zip_archive);
if (status == MagickFalse)
{
ThrowFileException(exception,CoderError,"UnableToReadImageData",
- read_info->filename);
+ read_info->filename);
(void) RelinquishUniqueFileResource(read_info->filename);
+ (void) fclose(file);
read_info=DestroyImageInfo(read_info);
image_metadata=DestroyImage(image_metadata);
return((Image *) NULL);
}
/* Delegate to ReadImage to read mergedimage.png */
+ read_info->file=file;
out_image=ReadImage(read_info,exception);
(void) RelinquishUniqueFileResource(read_info->filename);
read_info=DestroyImageInfo(read_info);
diff --git a/coders/pwp.c b/coders/pwp.c
index 3675a5e8c..0886480e9 100644
--- a/coders/pwp.c
+++ b/coders/pwp.c
@@ -216,7 +216,7 @@ static Image *ReadPWPImage(const ImageInfo *image_info,ExceptionInfo *exception)
*/
file=(FILE *) NULL;
if (unique_file != -1)
- file=fdopen(unique_file,"wb");
+ file=fdopen(unique_file,"rb+");
if ((unique_file == -1) || (file == (FILE *) NULL))
{
(void) RelinquishUniqueFileResource(filename);
@@ -237,13 +237,15 @@ static Image *ReadPWPImage(const ImageInfo *image_info,ExceptionInfo *exception)
if (fputc(c,file) != c)
break;
}
- (void) fclose(file);
+ if (fseek(file,0,SEEK_SET) != 0)
+ ThrowReaderException(FileOpenError,"UnableToCreateTemporaryFile");
if (c == EOF)
{
(void) RelinquishUniqueFileResource(filename);
read_info=DestroyImageInfo(read_info);
ThrowReaderException(CorruptImageError,"UnexpectedEndOfFile");
}
+ read_info->file=file;
next_image=ReadImage(read_info,exception);
if (next_image == (Image *) NULL)
break;
diff --git a/coders/sfw.c b/coders/sfw.c
index 083fb84a1..2f2992afb 100644
--- a/coders/sfw.c
+++ b/coders/sfw.c
@@ -330,7 +330,8 @@ static Image *ReadSFWImage(const ImageInfo *image_info,ExceptionInfo *exception)
extent=fwrite(HuffmanTable,1,sizeof(HuffmanTable)/sizeof(*HuffmanTable),file);
extent=fwrite(offset+1,1,(size_t) (data-offset),file);
status=ferror(file) != 0 ? MagickFalse : MagickTrue;
- (void) fclose(file);
+ if (fseek(file,0,SEEK_SET) != 0)
+ ThrowReaderException(FileOpenError,"UnableToCreateTemporaryFile");
(void) close_utf8(unique_file);
buffer=(unsigned char *) RelinquishMagickMemory(buffer);
if (status == MagickFalse)
@@ -338,6 +339,7 @@ static Image *ReadSFWImage(const ImageInfo *image_info,ExceptionInfo *exception)
char
*message;
+ (void) fclose(file);
(void) remove_utf8(read_info->filename);
read_info=DestroyImageInfo(read_info);
message=GetExceptionMessage(errno);
@@ -351,6 +353,7 @@ static Image *ReadSFWImage(const ImageInfo *image_info,ExceptionInfo *exception)
Read JPEG image.
*/
(void) CopyMagickString(read_info->magick,"JPEG",MagickPathExtent);
+ read_info->file=file;
jpeg_image=ReadImage(read_info,exception);
(void) RelinquishUniqueFileResource(read_info->filename);
read_info=DestroyImageInfo(read_info);
diff --git a/coders/svg.c b/coders/svg.c
index 302809a5e..1ff2cce7f 100644
--- a/coders/svg.c
+++ b/coders/svg.c
@@ -3157,7 +3157,7 @@ static Image *RenderMSVGImage(const ImageInfo *image_info,Image *image,
file=(FILE *) NULL;
unique_file=AcquireUniqueFileResource(filename);
if (unique_file != -1)
- file=fdopen(unique_file,"w");
+ file=fdopen(unique_file,"rb+");
if ((unique_file == -1) || (file == (FILE *) NULL))
{
(void) CopyMagickString(image->filename,filename,MagickPathExtent);
@@ -3255,7 +3255,8 @@ static Image *RenderMSVGImage(const ImageInfo *image_info,Image *image,
xmlFreeParserCtxt(parser);
if (image->debug != MagickFalse)
(void) LogMagickEvent(CoderEvent,GetMagickModule(),"end SAX");
- (void) fclose(file);
+ if (fseek(file,0,SEEK_SET) != 0)
+ ThrowReaderException(FileOpenError,"UnableToCreateTemporaryFile");
(void) CloseBlob(image);
image->columns=svg_info->width;
image->rows=svg_info->height;
@@ -3263,10 +3264,13 @@ static Image *RenderMSVGImage(const ImageInfo *image_info,Image *image,
{
svg_info=DestroySVGInfo(svg_info);
(void) RelinquishUniqueFileResource(filename);
+ (void) fclose(file);
image=DestroyImage(image);
return((Image *) NULL);
}
- if (image_info->ping == MagickFalse)
+ if (image_info->ping != MagickFalse)
+ (void) fclose(file);
+ else
{
ImageInfo
*read_info;
@@ -3278,6 +3282,7 @@ static Image *RenderMSVGImage(const ImageInfo *image_info,Image *image,
image=(Image *) NULL;
read_info=CloneImageInfo(image_info);
SetImageInfoBlob(read_info,(void *) NULL,0);
+ read_info->file=file;
(void) FormatLocaleString(read_info->filename,MagickPathExtent,"mvg:%s",
filename);
image=ReadImage(read_info,exception);
diff --git a/coders/wmf.c b/coders/wmf.c
index 4b8f8f178..23a0500c1 100644
--- a/coders/wmf.c
+++ b/coders/wmf.c
@@ -197,7 +197,7 @@ static Image *ReadWMFImage(const ImageInfo *image_info,ExceptionInfo *exception)
file=(FILE *) NULL;
unique_file=AcquireUniqueFileResource(filename);
if (unique_file != -1)
- file=fdopen(unique_file,"wb");
+ file=fdopen(unique_file,"rb+");
if ((unique_file == -1) || (file == (FILE *) NULL))
{
ipa_device_close(wmf_info);
@@ -213,7 +213,8 @@ static Image *ReadWMFImage(const ImageInfo *image_info,ExceptionInfo *exception)
wmf_api_destroy(wmf_info);
ThrowReaderException(DelegateError,"FailedToRenderFile");
}
- (void) fclose(file);
+ if (fseek(file,0,SEEK_SET) != 0)
+ ThrowImageException(FileOpenError,"UnableToCreateTemporaryFile");
wmf_api_destroy(wmf_info);
(void) CloseBlob(image);
image=DestroyImage(image);
@@ -221,6 +222,7 @@ static Image *ReadWMFImage(const ImageInfo *image_info,ExceptionInfo *exception)
Read EPS image.
*/
read_info=CloneImageInfo(image_info);
+ read_info->file=file;
SetImageInfoBlob(read_info,(void *) NULL,0);
(void) FormatLocaleString(read_info->filename,MagickPathExtent,"eps:%s",
filename);