Saving TIFF file is causing IccProfile and IptcProfile on existing file to be set to NULL

[JeffP134]

Prerequisites

• I have written a descriptive issue title
• I have verified that I am running the latest version of ImageSharp
• I have verified if the problem exist in both DEBUG and RELEASE mode
• I have searched open and closed issues to ensure it has not already been reported

ImageSharp version

3.0.1

Other ImageSharp packages and versions

None

Environment (Operating system, version and so on)

Windows 10

.NET Framework version

.NET 6

Description

I am using Visual Studio 2022 and .NET 6 to build an image metadata editor so I can edit multiple related image files at the same time.

I am able to load and edit both JPEG and TIFF metadata tags just fine on the ExifProfile sections of each format, and IptcProfile changes appear to be working prior to saving to disk.

However, upon saving back to the original file, the existing IccProfile and IptcProfile structures are being set to NULL in the image, resulting in a loss of information. ExifProfile and XmpProfile appear to be fine after the save, but I haven't inspected the XmpProfile closely to be sure.

This is also reducing the TIFF file size from 77 MB to 15 MB.

I was expecting the save to retain the IccProfile as is, and for the IptcProfile to be updated with my changes.

The following screenshots illustrate the issue.

TIFF Profiles before Editing ExifProfile and IptcProfile

TIFF Profiles after Editing but Before File Save

TIFF Profiles after Saving to Disk

File Size Pre-Edit

File Size After Save

Steps to Reproduce

1. Using ImageSharp's Image class, open a TIFF image file that has the IccProfile and IptcProfile properties of the RootFrame filled in at Image.Frames.RootFrame.Metadata
2. Edit the ExifProfile and IptcProfile sections of Image.Frames.RootFrame.Metadata to update metadata tags
3. Save the TIFF image to the hard drive
4. Open the saved TIFF image and check the IccProfile and IptcProfile properties of Image.Frames.RootFrame.Metadata

Images

The attached file is a smaller TIFF that only has the ExifProfile, IccProfile, and XmpProfile sections populated. In this scenario, my code creates and then populates the IptcProfile prior to saving.

But after the save, the file size reduces from 10 MB to 2 MB, and IccProfile and IptcProfile are both null.
Sunspot Region 3354 Colorized 2023-07-02 13-55.zip

[JimBobSquarePants]

It certainly looks like we're saving them.

ImageSharp/src/ImageSharp/Formats/Tiff/TiffEncoderEntriesCollector.cs

Lines 174 to 197 in 77d49e6

	if (!skipMetadata && imageMetadata.IptcProfile != null)
	{
	imageMetadata.IptcProfile.UpdateData();
	ExifByteArray iptc = new(ExifTagValue.IPTC, ExifDataType.Byte)
	{
	Value = imageMetadata.IptcProfile.Data
	};
	this.Collector.AddOrReplace(iptc);
	}
	else
	{
	exifProfile?.RemoveValue(ExifTag.IPTC);
	}
	if (imageMetadata.IccProfile != null)
	{
	ExifByteArray icc = new(ExifTagValue.IccProfile, ExifDataType.Undefined)
	{
	Value = imageMetadata.IccProfile.ToByteArray()
	};
	this.Collector.AddOrReplace(icc);
	}

Does the issue occur saving the images without editing?

[JeffP134]

Yes. I commented out all of the image editing and verified that right before the save, the metadata was all there. I then called the Save method on the Image instance:

image.Save(filenameAndPath);

and after the method call, the file size shrank. After reloading the image data, both profiles on the RootFrame are set to null.

I was able to successfully download the source code, add it to my solution as a project reference, and debug the logic. I think I found the bug. It is actually a problem in the ProcessProfiles method and the Process method that calls it on TiffEncoderEntriesCollector.

This will take a couple of screenshots to explain.

At this point, the image object has two instances of metadata. There is the outer metadata on the image class itself. This has the profiles all set to null.

But the metadata for the frame is on each frame instance in the image data structure:

The method that calls ProcessProfiles is passing in image.Metadata, and it is separately extracting image.rootframe.ExifProfile and image.rootFrame.XmpProfile to process separately. The IccProfile and IptcProfile on RootFrame are never used.

    public void Process(Image image, bool skipMetadata)
    {
        ImageFrame rootFrame = image.Frames.RootFrame;
        ExifProfile rootFrameExifProfile = rootFrame.Metadata.ExifProfile;
        XmpProfile rootFrameXmpProfile = rootFrame.Metadata.XmpProfile;
        
        this.ProcessProfiles(image.Metadata, skipMetadata, rootFrameExifProfile, rootFrameXmpProfile);
        
        if (!skipMetadata)
        {
            this.ProcessMetadata(rootFrameExifProfile ?? new ExifProfile());
        }
        
        if (!this.Collector.Entries.Exists(t => t.Tag == ExifTag.Software))
        {
            this.Collector.Add(new ExifString(ExifTagValue.Software)
            {
                Value = SoftwareValue
            });
        }
    }

When ProcessProfiles is called, it is actually using the ExifProfile and XmpProfile data from RootFrame and not from image.Metadata. The IccProfile and IptcProfile properties are null in image.Metadata, so they are being skipped.

I'll admit up front that I am not familiar with the Tiff format, so I don't know if image.Metadata is supposed to have null profiles outside of the individual frames, or if some metadata is allowed there.

I also don't know if each frame is allowed to have its own metadata. It would make sense to me if each frame can have its own metadata profiles, since each frame (I think) is supposed to be able to contain its own image in multi-image TIFFs. Each image in a multi-image TIFF should be able to have its own title, author, description, copyright information, color profile for printing, etc.

If the TIFF is supposed to have metadata outside of the frames, then this call in TiffEncoderCore's WriteFrame method would make sense to write out the metadata for the outer TIFF data structure:

    if (image != null)
    {
        entriesCollector.ProcessMetadata(image, this.skipMetadata);
    }

This call should only be handling that outer metadata and not using metadata defined on the frames. It might make sense to move that outside of the loop inside of TiffEncoderCore's Encode method:

    foreach (ImageFrame<TPixel> frame in image.Frames)
    {
        cancellationToken.ThrowIfCancellationRequested();
        
        ifdMarker = this.WriteFrame(writer, frame, image.Metadata, metadataImage, ifdMarker);
        metadataImage = null;
    }

rather than on each call to write the frame's data.

If each frame can have its own metadata, if present, then that if block inside of TiffEncoderCore's WriteFrame method should change to:

    if (frame.Metadata != null)
    {
        entriesCollector.ProcessMetadata(frame, this.skipMetadata);
    }

and new Process and ProcessProfiles methods should be added to TiffEncoderEntriesCollector to handle writing out the metadata for each frame and should not use the metadata at the image level or from image.RootFrame.

[JimBobSquarePants]

I wish everyone who raised issues made such a solid attempt to debug them as you! 🤩

I think your diagnosis of (and solution to) the issue is correct. As far as I understand Tiff can have global and per-frame metadata, so we have to make sure we encode both. Would you consider having a crack at contributing a fix?

[JeffP134]

I'll see if I can take a crack at contributing a fix for this. I spent part of this evening working out how to clone the repository, open it in Visual Studio, build the solution and run the tests. The plan was, after building and running the tests, I'd create a new local branch to work on the fix.

Unfortunately, I ran into an odd issue with the tests\Images folder. Somehow all of the images downloaded as corrupt. It also didn't matter if I used git to clone the repository or if I downloaded a zip file of the repository. The images in the tests\Images folder all appear to be corrupt.

For example, all of the JPEG files are not showing their images.

And if I try to open them in the Windows Image app, I'm seeing an error:

Git is showing the corrupt files as being valid for the repository. The bad images caused over 2300+ unit tests to fail.

To fix the issue, I'm having to bring up each image individually, download the raw file, and replace them in the tests\Images folder. I then rerun the tests to see how many are still broken.

Strangely, the fixed images are showing as different in GIT and are requesting to be pushed to the repository.

Any ideas how to fix this without having to download each file individually? I didn't see a download link on the tests\Images folder.

[JeffP134]

Never mind, I found the issue. I had to install GIT LFS first and enable long path names, then delete and re-clone the repository. The images downloaded correctly during cloning.

In case someone else runs into this issue, the correct order of steps for cloning the repository on Windows appears to be:

1. Enable long paths in GIT for Windows
2. Install GIT LFS
3. Restart the command prompt/PowerShell window
4. Verify GIT LFS was installed
5. Clone the repository, then check the files in tests\Images to make sure they downloaded correctly
6. Configure blame commands
7. Update submodules
8. Build the code
9. Run tests to verify they all pass
10. Create a new local branch from main to prepare for making your code changes

On another note, the Image file tests\Images\Input\Tiff\Issues\Issue1891.tiff triggered a Heuristic Virus alarm on my computer. (Specifically Bloodhound.Exploit.14). The file was automatically removed from my machine both in the folder and in GIT LFS.

I thought that I had better bring this to your attention, so you can check the file.

It is also the only failing test on my machine.

[JimBobSquarePants]

Sorry, was sleeping (time zones). Glad you figured it out.

Symantec is giving you a false positive there. The file is safe.
https://www.virustotal.com/gui/file/a2779c7fb2c2ad858e0d7efcfffd594cc6fb2846e0475a2998a3cda50f289b9b

See this related issue in the Pillow library where the same exploit is incorrectly detected in a tiff file.
python-pillow/Pillow#5725

Would it be possible for you to submit the file via https://symsubmit.symantec.com/ to get them to remove it from their lists? I'd do it myself but they're asking for detail I cannot provide since I don't use their product. Thanks!

[JeffP134]

Okay I think I have the fix ready to go. I modified the following files:

• src\ImageSharp\Fomats\Tiff\TiffEncoderCore.cs
• src\ImageSharp\Fomats\Tiff\TiffEncoderEntriesCollector.cs
• tests\ImageSharp.Tests\Formats\Tiff\TiffMetadataTests.cs - added 1 new test

I also modified and then removed a change to TestImages.cs. I thought I was going to need a new image but figured out how to do the entire test using code only. I also ran all existing tests to make sure the code changes didn't break them.

I was not able to move the if statement outside of the frame loop, so I left it where it was at.

Unfortunately, I can't push the branch from Visual Studio to Github due to a permission denied issue.

[JimBobSquarePants]

What was the message in the permissions?

It’s likely due to changes in Git LFS due to the misreporting of a virus in that Tiff file.

What does ‘git status’ report?

[JeffP134]

Oh, and I did submit that image to Symantec. I also found a way to restore the file so it was not one of the files that was indicating a change by GIT.

Here's the logged messages from GIT:

Undoing delete: D:\GitHubProjects\ImageSharp\tests\Images\Input\Tiff\Issues\Issue1891.tiff
Commit 81d30453 created locally in repository D:\GitHubProjects\ImageSharp
Pushing fix-2504
Remote: Permission to SixLabors/ImageSharp.git denied to JeffP134.
Error encountered while pushing to the remote repository: Git failed with a fatal error.
Git failed with a fatal error.
unable to access 'https://github.com/SixLabors/ImageSharp/': The requested URL returned error: 403

Failed to push to the remote repository. See the Output window for more details.
Commit 2344be0d created locally in repository D:\GitHubProjects\ImageSharp

And here's what git status reported:

On branch fix-2504
nothing to commit, working tree clean

The local repository compare for GIT is showing these changes:

This branch currently only exists locally.

[tocsoft]

@JeffP134 you need to fork the repo first and push your changes to your fork, then you can create a PR from your fork back to this repo.

[JimBobSquarePants]

https://docs.github.com/en/get-started/quickstart/fork-a-repo

[JeffP134]

Sorry about that, first time contributing to anything on Github.

[JimBobSquarePants]

No need to apologize, we're grateful for your effort so far!