Skip to content
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.

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

exiv2 corrupts Sony 2010e tags when writing other metadata #1471

Closed
akobel opened this issue Feb 13, 2021 · 13 comments
Closed

exiv2 corrupts Sony 2010e tags when writing other metadata #1471

akobel opened this issue Feb 13, 2021 · 13 comments
Assignees
Labels
Milestone

Comments

@akobel
Copy link

akobel commented Feb 13, 2021

Excerpt from #1464 (comment) case 2:
This is still buggy after PR #1469.

mkdir -p ~/temp
curl https://user-images.githubusercontent.com/4997431/106392842-090dc700-63f4-11eb-856e-749f697bd64d.jpg >! /temp/1464.jpg 2>&-
./exiv2 -pa --grep 2010e.LensType2 ~/temp/1464.jpg
./exiv2 -M'set Exif.Image.ImageDescription Ascii 01234567890123456789012345678901' ~/temp/1464.jpg
./exiv2 -pa --grep 2010e.LensType2 ~/temp/1464.jpg

yields on 0.27-maintenance:

Exif.Sony2010e.LensType2                     Short       1  1024   #  <-- correct
Exif.Sony2010e.LensType2                     Short       1  51712  #  <-- wrong

No corruption if the new ImageDescription is shorter than 32 characters (and fits in the already allocated tag); see case 1 in the earlier bug report.
The same corruption with ./exiv2 -M'set Exif.Image.ImageID Ascii foobar' ~/temp/1464.jpg to add a new tag; see case 3 in the earlier bug report.

On master, Sony2010e tags are not printed, but exiftool (e.g., via exiftool -v ~/temp/1464.jpg | grep WB_RGB) confirms that the values are not corrupted.

@akobel akobel added the bug label Feb 13, 2021
@clanmills
Copy link
Collaborator

clanmills commented Feb 13, 2021

Thanks for reporting this, Alex. We can safely ignore matters concerning Sony tag 0x2010 on master because it is ignored. It's read-only.

However it's important to resolve this on the 0.27-maintenance branch as that should will one-day be in master. It's curious that the length of ImageDescription is implicated in this matter. Groan. I think next week's gonna be another long week.

@clanmills clanmills self-assigned this Feb 14, 2021
@clanmills clanmills added this to the v0.27.4 milestone Feb 14, 2021
@clanmills
Copy link
Collaborator

I think there are two parts to this report.

1 Sony2010e.LensType2 corrupted by modifying Exif.Image.ImageDescription > 32 bytes

1076 rmills@rmillsmm-local:~/temp $ curl --silent -O https://user-images.githubusercontent.com/4997431/106392842-090dc700-63f4-11eb-856e-749f697bd64d.jpg
1077 rmills@rmillsmm-local:~/temp $ exiv2 -pa --grep 2010e.LensType2 --grep description/i 106392842-090dc700-63f4-11eb-856e-749f697bd64d.jpg 
Exif.Image.ImageDescription                  Ascii      32                                 
Exif.Sony2010e.LensType2                     Short       1  1024
Exif.Thumbnail.ImageDescription              Ascii      32                                 
1078 rmills@rmillsmm-local:~/temp $ exiv2 -M'set Exif.Image.ImageDescription Ascii 0123456' 106392842-090dc700-63f4-11eb-856e-749f697bd64d.jpg 
1079 rmills@rmillsmm-local:~/temp $ exiv2 -pa --grep 2010e.LensType2 --grep description/i 106392842-090dc700-63f4-11eb-856e-749f697bd64d.jpg 
Exif.Image.ImageDescription                  Ascii       8  0123456
Exif.Sony2010e.LensType2                     Short       1  1024
Exif.Thumbnail.ImageDescription              Ascii      32                                 
1080 rmills@rmillsmm-local:~/temp $ exiv2 -M'set Exif.Image.ImageDescription Ascii 01234567890123456789012345678901234567890' 106392842-090dc700-63f4-11eb-856e-749f697bd64d.jpg 
1081 rmills@rmillsmm-local:~/temp $ exiv2 -pa --grep 2010e.LensType2 --grep description/i 106392842-090dc700-63f4-11eb-856e-749f697bd64d.jpg 
Exif.Image.ImageDescription                  Ascii      42  01234567890123456789012345678901234567890
Exif.Sony2010e.LensType2                     Short       1  51712
Exif.Thumbnail.ImageDescription              Ascii      32                                 
1082 rmills@rmillsmm-local:~/temp $ 

2 Something to do with foobar

I haven't understood this.

I will focus on the > 32 bytes case for now. When that's solved, perhaps you could explore and explain the foobar case.

@clanmills
Copy link
Collaborator

OK. The whole of the Sony2010e tag is corrupted when the makernote is being relocated in the file. It's not only LensType2, the WB_ values are also corrupted. Here's the evidence:

1117 rmills@rmillsmm-local:~/temp $ curl --silent -O https://user-images.githubusercontent.com/4997431/106392842-090dc700-63f4-11eb-856e-749f697bd64d.jpg
1118 rmills@rmillsmm-local:~/temp $ exiv2 -pa --grep 2010e.WB 106392842-090dc700-63f4-11eb-856e-749f697bd64d.jpg
Exif.Sony2010e.WB_RGBLevels                  Short       3  598 256 442
1119 rmills@rmillsmm-local:~/temp $ exiv2 -M'set Exif.Image.ImageDescription Ascii 0123456' 106392842-090dc700-63f4-11eb-856e-749f697bd64d.jpg
1120 rmills@rmillsmm-local:~/temp $ exiv2 -pa --grep 2010e.WB 106392842-090dc700-63f4-11eb-856e-749f697bd64d.jpg
Exif.Sony2010e.WB_RGBLevels                  Short       3  598 256 442
1121 rmills@rmillsmm-local:~/temp $ exiv2 -M'set Exif.Image.ImageDescription Ascii 0123456789012345678901234567890' 106392842-090dc700-63f4-11eb-856e-749f697bd64d.jpg
1122 rmills@rmillsmm-local:~/temp $ exiv2 -pa --grep 2010e.WB 106392842-090dc700-63f4-11eb-856e-749f697bd64d.jpg
Exif.Sony2010e.WB_RGBLevels                  Short       3  22957 256 295
1123 rmills@rmillsmm-local:~/temp $ 

When I submitted #1469, I wondered about making 0x2010 read only. The code to achieve that is:

diff --git a/src/tiffimage_int.cpp b/src/tiffimage_int.cpp
index 90e5d390..54509db8 100644
--- a/src/tiffimage_int.cpp
+++ b/src/tiffimage_int.cpp
@@ -2035,6 +2035,7 @@ namespace Exiv2 {
             { 0x8828, ifd0Id }, // Exif.Image.OECF
             { 0x9102, ifd0Id }, // Exif.Image.CompressedBitsPerPixel
             { 0x9217, ifd0Id }, // Exif.Image.SensingMethod
+            { 0x2010, sony2010eId }, // Exif.Exif.Sony2010e
         };
 
         // If tag, group is one of the image tags listed above -> bingo!
1142 rmills@rmillsmm-local:~/gnu/github/exiv2/0.27-maintenance $ 

And that fixes it. Of course, it will break the test suite and I'll deal with that later.

Perhaps you could help me to understand the "foobar" issue and hopefully we can deal with that today. Today is Valentine's Day. Yesterday, Alison and I took our 15 year old cat Lizzie to the vet and had her put to sleep. We're heart broken. Working on Exiv2 bugs is an escape from how we feel today.

1143 rmills@rmillsmm-local:~/temp $ curl --silent -O https://user-images.githubusercontent.com/4997431/106392842-090dc700-63f4-11eb-856e-749f697bd64d.jpg
1144 rmills@rmillsmm-local:~/temp $ exiv2 -pa --grep 2010e --grep desc/i 106392842-090dc700-63f4-11eb-856e-749f697bd64d.jpg
Exif.Image.ImageDescription                  Ascii      32                                 
Exif.Sony2010e.SequenceImageNumber           Long        1  0
Exif.Sony2010e.SequenceFileNumber            Long        1  0
Exif.Sony2010e.ReleaseMode2                  Long        1  26
Exif.Sony2010e.DigitalZoomRatio              Byte        1  16
Exif.Sony2010e.SonyDateTime                  Undefined   1  228
Exif.Sony2010e.DynamicRangeOptimizer         Byte        1  1
Exif.Sony2010e.MeterInfo                     Undefined   1  12
Exif.Sony2010e.ReleaseMode3                  Byte        1  1
Exif.Sony2010e.ReleaseMode2                  Byte        1  26
Exif.Sony2010e.SelfTimer                     Byte        1  0
Exif.Sony2010e.FlashMode                     Byte        1  2
Exif.Sony2010e.StopsAboveBaseISO             Short       1  3797
Exif.Sony2010e.BrightnessValue               Short       1  15966
Exif.Sony2010e.DynamicRangeOptimizer         Byte        1  1
Exif.Sony2010e.HDRSetting                    Byte        1  0
Exif.Sony2010e.ExposureCompensation          SShort      1  0
Exif.Sony2010e.PictureProfile                Byte        1  0
Exif.Sony2010e.PictureProfile2               Byte        1  0
Exif.Sony2010e.PictureEffect2                Byte        1  0
Exif.Sony2010e.Quality2                      Byte        1  2
Exif.Sony2010e.MeteringMode                  Byte        1  0
Exif.Sony2010e.ExposureProgram               Byte        1  2
Exif.Sony2010e.WB_RGBLevels                  Short       3  598 256 442
Exif.Sony2010e.SonyISO                       Short       1  3711
Exif.Sony2010e.SonyISO2                      Short       1  642
Exif.Sony2010e.FocalLength                   Short       1  33794
Exif.Sony2010e.MinFocalLength                Short       1  33059
Exif.Sony2010e.MaxFocalLength                Short       1  35328
Exif.Sony2010e.SonyISO3                      Short       1  13312
Exif.Sony2010e.DistortionCorrParams          SShort     16  0 0 0 0 0 0 1 0 1 0 4 0 24 0 64 0
Exif.Sony2010e.LensFormat                    Byte        1  0
Exif.Sony2010e.LensMount                     Byte        1  0
Exif.Sony2010e.LensType2                     Short       1  1024
Exif.Sony2010e.LensType                      Short       1  0
Exif.Sony2010e.DistortionCorrParamsPresent   Byte        1  24
Exif.Sony2010e.DistortionCorrParamsNumber    Byte        1  0
Exif.Thumbnail.ImageDescription              Ascii      32                                 
1145 rmills@rmillsmm-local:~/temp $ exiv2 -M'set Exif.Image.ImageDescription Ascii 0123456789012345678901234567890' 106392842-090dc700-63f4-11eb-856e-749f697bd64d.jpg
1146 rmills@rmillsmm-local:~/temp $ exiv2 -pa --grep 2010e --grep desc/i 106392842-090dc700-63f4-11eb-856e-749f697bd64d.jpg
Exif.Image.ImageDescription                  Ascii      32  0123456789012345678901234567890
Exif.Sony2010e.SequenceImageNumber           Long        1  0
Exif.Sony2010e.SequenceFileNumber            Long        1  0
Exif.Sony2010e.ReleaseMode2                  Long        1  26
Exif.Sony2010e.DigitalZoomRatio              Byte        1  16
Exif.Sony2010e.SonyDateTime                  Undefined   1  228
Exif.Sony2010e.DynamicRangeOptimizer         Byte        1  1
Exif.Sony2010e.MeterInfo                     Undefined   1  12
Exif.Sony2010e.ReleaseMode3                  Byte        1  1
Exif.Sony2010e.ReleaseMode2                  Byte        1  26
Exif.Sony2010e.SelfTimer                     Byte        1  0
Exif.Sony2010e.FlashMode                     Byte        1  2
Exif.Sony2010e.StopsAboveBaseISO             Short       1  3797
Exif.Sony2010e.BrightnessValue               Short       1  15966
Exif.Sony2010e.DynamicRangeOptimizer         Byte        1  1
Exif.Sony2010e.HDRSetting                    Byte        1  0
Exif.Sony2010e.ExposureCompensation          SShort      1  0
Exif.Sony2010e.PictureProfile                Byte        1  0
Exif.Sony2010e.PictureProfile2               Byte        1  0
Exif.Sony2010e.PictureEffect2                Byte        1  0
Exif.Sony2010e.Quality2                      Byte        1  2
Exif.Sony2010e.MeteringMode                  Byte        1  0
Exif.Sony2010e.ExposureProgram               Byte        1  2
Exif.Sony2010e.WB_RGBLevels                  Short       3  598 256 442
Exif.Sony2010e.SonyISO                       Short       1  3711
Exif.Sony2010e.SonyISO2                      Short       1  642
Exif.Sony2010e.FocalLength                   Short       1  33794
Exif.Sony2010e.MinFocalLength                Short       1  33059
Exif.Sony2010e.MaxFocalLength                Short       1  35328
Exif.Sony2010e.SonyISO3                      Short       1  13312
Exif.Sony2010e.DistortionCorrParams          SShort     16  0 0 0 0 0 0 1 0 1 0 4 0 24 0 64 0
Exif.Sony2010e.LensFormat                    Byte        1  0
Exif.Sony2010e.LensMount                     Byte        1  0
Exif.Sony2010e.LensType2                     Short       1  1024
Exif.Sony2010e.LensType                      Short       1  0
Exif.Sony2010e.DistortionCorrParamsPresent   Byte        1  24
Exif.Sony2010e.DistortionCorrParamsNumber    Byte        1  0
Exif.Thumbnail.ImageDescription              Ascii      32                                 
1147 rmills@rmillsmm-local:~/temp $ 

@akobel
Copy link
Author

akobel commented Feb 14, 2021

Robin,
first, my condolences to your family on the loss of your loved cat, and thanks for taking the time nevertheless to work on such things. I hope the distraction helps.

Concerning the foobar: there's nothing special about the string, but note that this example writes to ImageID rather than ImageDescription. If I understand correctly, ImageID is not already an entry of the original tag set. Hence, this seems to be an instantiation of the exact same behavior as increasing the length of ImageDescription: On adding the new tag, or extending an existing tag beyond the already allocated space, the other tags (notably including Makernote, as the 2010e range seems to be encoded in there?) shift to a new location.

Now, I'm not familiar with the Makernote structure - is there an offset à la "2010e starts at address 0x01234567" somewhere? If so, shifting the Makernote without adjusting this tag would explain the observations.

@akobel
Copy link
Author

akobel commented Feb 14, 2021

But, for what it's worth: if this is the sole explanation, I would expect a 0x00ca (little-endian for 51712) in close vicinity of 0x0004 (little-endian for 1024). Most likely at a distance of 32 byte or a multiple thereof. But I couldn't spot such an occurence...

@clanmills
Copy link
Collaborator

clanmills commented Feb 14, 2021

Thank You for your kind remarks about Lizzie.

The MakerNote is an IFD. An IFD is an array of tag/offsets. And now, I understand the foobar issue is the makernote moved issue. When we add an extra tag EG: Exif.Image.ImageID the makernote moves and the 0x2010 tag is corrupted (even when Exif.Sony2010e is read-only).

This is the same error that we are seeing in #1472 when we use the in command to copy the makernote from the .exv to the .jpg.

I'm going to have to do the hard work of debugging the makernote encoder/decoder. I suspect it will be something simple like the cipher isn't being called. However, it's going to take more effort than my enthusiasm today. I'll work on this later in the week when I don't have a hang-over!

The upside of fixing this is that we will retain write capability for 0x2010. The MakerNote code in Exiv2 is able to read/modify/write tags, so it ought to work. The 0x2010 tag is challenging because it uses the Sony cipher. I'll fix it. I always do.

1178 rmills@rmillsmm-local:~/temp $ exiv2 --grep 2010 106392842-090dc700-63f4-11eb-856e-749f697bd64d.jpg 
Exif.Sony2010e.SequenceImageNumber           Long        1  0
Exif.Sony2010e.SequenceFileNumber            Long        1  0
Exif.Sony2010e.ReleaseMode2                  Long        1  26
Exif.Sony2010e.DigitalZoomRatio              Byte        1  16
Exif.Sony2010e.SonyDateTime                  Undefined   1  228
Exif.Sony2010e.DynamicRangeOptimizer         Byte        1  1
Exif.Sony2010e.MeterInfo                     Undefined   1  12
Exif.Sony2010e.ReleaseMode3                  Byte        1  1
Exif.Sony2010e.ReleaseMode2                  Byte        1  26
Exif.Sony2010e.SelfTimer                     Byte        1  0
Exif.Sony2010e.FlashMode                     Byte        1  2
Exif.Sony2010e.StopsAboveBaseISO             Short       1  3797
Exif.Sony2010e.BrightnessValue               Short       1  15966
Exif.Sony2010e.DynamicRangeOptimizer         Byte        1  1
Exif.Sony2010e.HDRSetting                    Byte        1  0
Exif.Sony2010e.ExposureCompensation          SShort      1  0
Exif.Sony2010e.PictureProfile                Byte        1  0
Exif.Sony2010e.PictureProfile2               Byte        1  0
Exif.Sony2010e.PictureEffect2                Byte        1  0
Exif.Sony2010e.Quality2                      Byte        1  2
Exif.Sony2010e.MeteringMode                  Byte        1  0
Exif.Sony2010e.ExposureProgram               Byte        1  2
Exif.Sony2010e.WB_RGBLevels                  Short       3  598 256 442
Exif.Sony2010e.SonyISO                       Short       1  3711
Exif.Sony2010e.SonyISO2                      Short       1  642
Exif.Sony2010e.FocalLength                   Short       1  33794
Exif.Sony2010e.MinFocalLength                Short       1  33059
Exif.Sony2010e.MaxFocalLength                Short       1  35328
Exif.Sony2010e.SonyISO3                      Short       1  13312
Exif.Sony2010e.DistortionCorrParams          SShort     16  0 0 0 0 0 0 1 0 1 0 4 0 24 0 64 0
Exif.Sony2010e.LensFormat                    Byte        1  0
Exif.Sony2010e.LensMount                     Byte        1  0
Exif.Sony2010e.LensType2                     Short       1  1024
Exif.Sony2010e.LensType                      Short       1  0
Exif.Sony2010e.DistortionCorrParamsPresent   Byte        1  24
Exif.Sony2010e.DistortionCorrParamsNumber    Byte        1  0
1179 rmills@rmillsmm-local:~/temp $ exiv2 -M'set Exif.Image.ImageID foobar' 106392842-090dc700-63f4-11eb-856e-749f697bd64d.jpg 
1180 rmills@rmillsmm-local:~/temp $ exiv2 --grep 2010 106392842-090dc700-63f4-11eb-856e-749f697bd64d.jpg 
Exif.Sony2010e.SequenceImageNumber           Long        1  0
Exif.Sony2010e.SequenceFileNumber            Long        1  0
Exif.Sony2010e.ReleaseMode2                  Long        1  236
Exif.Sony2010e.DigitalZoomRatio              Byte        1  217
Exif.Sony2010e.SonyDateTime                  Undefined   1  219
Exif.Sony2010e.DynamicRangeOptimizer         Byte        1  1
Exif.Sony2010e.MeterInfo                     Undefined   1  3
Exif.Sony2010e.ReleaseMode3                  Byte        1  1
Exif.Sony2010e.ReleaseMode2                  Byte        1  236
Exif.Sony2010e.SelfTimer                     Byte        1  0
Exif.Sony2010e.FlashMode                     Byte        1  89
Exif.Sony2010e.StopsAboveBaseISO             Short       1  740
Exif.Sony2010e.BrightnessValue               Short       1  13608
Exif.Sony2010e.DynamicRangeOptimizer         Byte        1  1
Exif.Sony2010e.HDRSetting                    Byte        1  0
Exif.Sony2010e.ExposureCompensation          SShort      1  0
Exif.Sony2010e.PictureProfile                Byte        1  0
Exif.Sony2010e.PictureProfile2               Byte        1  0
Exif.Sony2010e.PictureEffect2                Byte        1  0
Exif.Sony2010e.Quality2                      Byte        1  89
Exif.Sony2010e.MeteringMode                  Byte        1  0
Exif.Sony2010e.ExposureProgram               Byte        1  89
Exif.Sony2010e.WB_RGBLevels                  Short       3  22957 256 295
Exif.Sony2010e.SonyISO                       Short       1  624
Exif.Sony2010e.SonyISO2                      Short       1  22929
Exif.Sony2010e.FocalLength                   Short       1  30809
Exif.Sony2010e.MinFocalLength                Short       1  27104
Exif.Sony2010e.MaxFocalLength                Short       1  60672
Exif.Sony2010e.SonyISO3                      Short       1  22528
Exif.Sony2010e.DistortionCorrParams          SShort     16  0 0 0 0 0 0 1 0 1 0 202 0 18 0 10 0
Exif.Sony2010e.LensFormat                    Byte        1  0
Exif.Sony2010e.LensMount                     Byte        1  0
Exif.Sony2010e.LensType2                     Short       1  51712
Exif.Sony2010e.LensType                      Short       1  0
Exif.Sony2010e.DistortionCorrParamsPresent   Byte        1  18
Exif.Sony2010e.DistortionCorrParamsNumber    Byte        1  0
1181 rmills@rmillsmm-local:~/temp $ 

This is the last photo of Lizzie. We took it on Tuesday evening and sent it to let the family know that she only had a few more days.
Lizzie_2021-02-09_23 37 38

@clanmills
Copy link
Collaborator

clanmills commented Feb 14, 2021

I'm going to solve this on Monday AND work my way through several classes that I haven't documented in the book:

class TiffEncoder : TiffVisitor {};
class TiffDecoder : TiffVisitor {};
class TiffCopier : TiffVisitor {};

I think the code is equally broken on 'master' and '0.27-maintenance'. The big change in 0.27-maintenance is that it reports tag 0x2010 and now we can see there is something wrong! And we've fixed the endian issue when we write 0x2010

In the original file, the MakerNote is of length 25717 bytes at offset 832. When we write Exif.Image.ImageID, the makernote moves and is of length 25710 bytes at offset 852. And that behaviour is identical in master and 0.27-maintenance.

So, question 1 is "why has the length changed?" when we haven't changed the MakerNote.

The 0x2010 tag is of length 6796 bytes and moved from offset 14489 to 14502. From the limited bytes in the dump of 0x2010, the encrypted/decrypted is different. We would expect 0x2010 to be different because we've fixed the endian issue.

The question 2 has to be "which bytes have changed in the makernote and why?".

Monday's going to be an interesting day.

Setup Environment

cd ~/temp
rm -rf *.jpg
curl --silent -O https://user-images.githubusercontent.com/4997431/106392842-090dc700-63f4-11eb-856e-749f697bd64d.jpg
cp 106392842-090dc700-63f4-11eb-856e-749f697bd64d.jpg 1464.jpg
cp 1464.jpg master.jpg
cp 1464.jpg 0.27-maintenance.jpg
ls -l *.jpg
tvisitor -pRU 1464.jpg | grep -e MakerNote -e 2010 -e tag
     address |    tag                                  |      type |    count |    offset | value
         504 | 0x927c Exif.Photo.MakerNote             | UNDEFINED |    25717 |       832 | SONY DSC ___N_..._.___..___ ._._____ +++
          1746 | 0x2010 Exif.Sony.Tagx2010               | UNDEFINED |     6796 |     14489 | ________._______pq..____.______6__J. +++
          1746 | 0x2010 Exif.Sony.Tagx2010               | UNDEFINED |     6796 |     14489 | ________._______....____.______.__.. +++
         19021 | 0x2010 Exif.Sony.WB_RGBLevels           |     SHORT |        3 |           | 598 256 442

master branch

1250 rmills@rmillsmm-local:~/temp $ exiv2 -M'set Exif.Image.ImageID foobar' master.jpg
1251 rmills@rmillsmm-local:~/temp $ tvisitor -pRU master.jpg | grep -e MakerNote -e 2010 -e tag
     address |    tag                                  |      type |    count |    offset | value
         524 | 0x927c Exif.Photo.MakerNote             | UNDEFINED |    25710 |       852 | SONY DSC ___N_..._.___..___ ._._____ +++
          1766 | 0x2010 Exif.Sony.Tagx2010               | UNDEFINED |     6796 |     14502 | ________._______pq..____.______6__J. +++
          1766 | 0x2010 Exif.Sony.Tagx2010               | UNDEFINED |     6796 |     14502 | ________._______....____.______.__.. +++
         19034 | 0x2010 Exif.Sony.WB_RGBLevels           |     SHORT |        3 |           | 598 256 442

0.27-maintenance branch

1258 rmills@rmillsmm-local:~/temp $ exiv2 -M'set Exif.Image.ImageID foobar' 0.27-maintenance.jpg 
1259 rmills@rmillsmm-local:~/temp $ tvisitor -pRU 0.27-maintenance.jpg | grep -e MakerNote -e 2010 -e tag
     address |    tag                                  |      type |    count |    offset | value
         524 | 0x927c Exif.Photo.MakerNote             | UNDEFINED |    25710 |       852 | SONY DSC ___N_..._.___..___ ._._____ +++
       address |    tag                                  |      type |    count |    offset | value
          1766 | 0x2010 Exif.Sony.Tagx2010               | UNDEFINED |     6796 |     14502 | ________,_______d..c____.______B__.. +++
          1766 | 0x2010 Exif.Sony.Tagx2010               | UNDEFINED |     6796 |     14502 | ________._______..".____.______.__.. +++
         19034 | 0x2010 Exif.Sony.WB_RGBLevels           |     SHORT |        3 |           | 22957 256 295

@clanmills
Copy link
Collaborator

clanmills commented Feb 15, 2021

I've set up the debugger to:

1 shell: cp ~/temp/1464.jpg ~/temp/0.27-maintenance.jpg
2 debug: exiv2 -M'set Exif.Image.ImageID foobar' ~/temp/0.27-maintenance.jpg

We call the sony cipher from a recursion invoked by src/tiffimage_int.cpp. We correctly take the writeMethod == wmIntrusive branch.

We never call the sony cipher again. I think TiffCopier may be making a mistake because 0x2010 is ciphered. I'm going to focus the 0x2010 tag in 0.27-maintenance.jpg:12->33769 at offset 14502. I have a stand-alone version of the sony cipher code (in another issue report).

    WriteMethod TiffParserWorker::encode(
              BasicIo&           io,
        const byte*              pData,
              uint32_t           size,
        const ExifData&          exifData,
        const IptcData&          iptcData,
        const XmpData&           xmpData,
              uint32_t           root,
              FindEncoderFct     findEncoderFct,
              TiffHeaderBase*    pHeader,
              OffsetWriter*      pOffsetWriter
    )
    {
        /*
           1) parse the binary image, if one is provided, and
           2) attempt updating the parsed tree in-place ("non-intrusive writing")
           3) else, create a new tree and write a new TIFF structure ("intrusive
              writing"). If there is a parsed tree, it is only used to access the
              image data in this case.
         */
        assert(pHeader);
        assert(pHeader->byteOrder() != invalidByteOrder);
        WriteMethod writeMethod = wmIntrusive;
        TiffComponent::AutoPtr parsedTree = parse(pData, size, root, pHeader);
        PrimaryGroups primaryGroups;
        findPrimaryGroups(primaryGroups, parsedTree.get());
        if (0 != parsedTree.get()) {
            // Attempt to update existing TIFF components based on metadata entries
            TiffEncoder encoder(exifData,
                                iptcData,
                                xmpData,
                                parsedTree.get(),
                                false,
                                &primaryGroups,
                                pHeader,
                                findEncoderFct);
            parsedTree->accept(encoder);
            if (!encoder.dirty()) writeMethod = wmNonIntrusive;
        }
        if (writeMethod == wmIntrusive) {
            TiffComponent::AutoPtr createdTree = TiffCreator::create(root, ifdIdNotSet);
            if (0 != parsedTree.get()) {
                // Copy image tags from the original image to the composite
                TiffCopier copier(createdTree.get(), root, pHeader, &primaryGroups);
                parsedTree->accept(copier);
            }
            // Add entries from metadata to composite
            TiffEncoder encoder(exifData,
                                iptcData,
                                xmpData,
                                createdTree.get(),
                                parsedTree.get() == 0,
                                &primaryGroups,
                                pHeader,
                                findEncoderFct);
            encoder.add(createdTree.get(), parsedTree.get(), root);
            // Write binary representation from the composite tree
            DataBuf header = pHeader->write();
            BasicIo::AutoPtr tempIo(new MemIo);
            assert(tempIo.get() != 0);
            IoWrapper ioWrapper(*tempIo, header.pData_, header.size_, pOffsetWriter);
            uint32_t imageIdx(uint32_t(-1));
            createdTree->write(ioWrapper,
                               pHeader->byteOrder(),
                               header.size_,
                               uint32_t(-1),
                               uint32_t(-1),
                               imageIdx);
            if (pOffsetWriter) pOffsetWriter->writeOffsets(*tempIo);
            io.transfer(*tempIo); // may throw
#ifndef SUPPRESS_WARNINGS
            EXV_INFO << "Write strategy: Intrusive\n";
#endif
        }
#ifndef SUPPRESS_WARNINGS
        else {
            EXV_INFO << "Write strategy: Non-intrusive\n";
        }
#endif
        return writeMethod;
    } // TiffParserWorker::encode

@clanmills
Copy link
Collaborator

I put code into sonyTagEncipher() to write files of the data before/after ciphering. There is no question that the data is correctly ciphered:

1338 rmills@rmillsmm-local:~/temp $ tvisitor -pR 1464.jpg | grep 2010
          1746 | 0x2010 Exif.Sony.Tagx2010               | UNDEFINED |     6796 |     14489 | ________._______pq..____.______6__J. +++
          1746 | 0x2010 Exif.Sony.Tagx2010               | UNDEFINED |     6796 |     14489 | ________._______....____.______.__.. +++
         19021 | 0x2010 Exif.Sony.WB_RGBLevels           |     SHORT |        3 |           | 598 256 442
1342 rmills@rmillsmm-local:~/temp $ dmpf 6796.out count=44 width=50
       0        0: ________._______pq..____.______6__J.'C$?____        ->  00 00 00 00 00 00 00 00 92 00 00 00 00 00 00 00 70 71 b8 99 00 00 00 00 01 00 00 00 00 00 00 36 00 00 4a 01 27 43 24 3f 00 00 00 00
1343 rmills@rmillsmm-local:~/temp $ 

However that's not what is stored in the 0.27-maintenance.jpg!

1350 rmills@rmillsmm-local:~/temp $ tvisitor -pR 0.27-maintenance.jpg | grep 2010 
          1766 | 0x2010 Exif.Sony.Tagx2010               | UNDEFINED |     6796 |     14502 | ________,_______d..c____.______B__.. +++
          1766 | 0x2010 Exif.Sony.Tagx2010               | UNDEFINED |     6796 |     14502 | ________._______..".____.______.__.. +++
         19034 | 0x2010 Exif.Sony.WB_RGBLevels           |     SHORT |        3 |           | 22957 256 295
1351 rmills@rmillsmm-local:~/temp $

I think there is something wrong here. Encoder does not write 0x2010 again.

            // Add entries from metadata to composite
            TiffEncoder encoder(exifData,
                                iptcData,
                                xmpData,
                                createdTree.get(),
                                parsedTree.get() == 0,
                                &primaryGroups,
                                pHeader,
                                findEncoderFct);

How was the binary data __d..c__ created? Has he retained data provided by the Copier? Don't know yet.

@clanmills
Copy link
Collaborator

clanmills commented Feb 15, 2021

I'm close. The TiffCopier/TiffEncoder dance used by writeMethod==wmIntrusive creates the tag 0x2010 in memory without knowing it must be ciphered. The tag is manufactured by this code:

encoder.add(createdTree.get(), parsedTree.get(), root);

We need to call sonyTagEncipher() on the newly minted tag. We must not do that for writeMethod == wmNonIntrusive.

We should also pay attention to 0x9402 Exif.Sony.FocalPosition which also uses the sony cipher. So his metadata is probably being damaged in a similar way.

@clanmills
Copy link
Collaborator

clanmills commented Feb 15, 2021

This is the most difficult issue I've ever encountered in Exiv2. Before this, the change to add the SonyCipher to support AutoFocus tags was the most complex change #582. This change adds an additional layer of complication to that change.

The data is correctly ciphered on the first call to the TiffEncoder. Because everything's being moved, we do the TiffCopier/TiffEncoder dance. I don't know exactly what the TiffCopier does. I think it preserves some data, the TiffEncoder is called again to produce a new Tiff, and then preserved data in the copier is merged back in. Maybe all we have to save the 0x2010 binary tag (which was correctly enciphered), then it will be correctly merged into the new Tiff.

The challenge is that the binary value of 0x2010 must be parsed by the MakerNote decoder. How is that possible? That's a complex operation for which there isn't an API.

I suspect the fix is to copy the ciphered bytes to the new tiff in/after encoder.add(). Something like:

if (writeMethod == wmIntrusive) {

    DataBuf buff0x2010 = something;

    TiffComponent::AutoPtr createdTree = TiffCreator::create(root, ifdIdNotSet);
    if (0 != parsedTree.get()) {
        // Copy image tags from the original image to the composite
        TiffCopier copier(createdTree.get(), root, pHeader, &primaryGroups);
        parsedTree->accept(copier);
    }
    // Add entries from metadata to composite
    TiffEncoder encoder(exifData,
                        iptcData,
                        xmpData,
                        createdTree.get(),
                        parsedTree.get() == 0,
                        &primaryGroups,
                        pHeader,
                        findEncoderFct);
    encoder.add(createdTree.get(), parsedTree.get(), root);

    something(buff0x2010);

    // Write binary representation from the composite tree
    DataBuf header = pHeader->write();
    ...
}

I'm going to break and stop working on this for now. The primary feature of Exiv2 v0.27.4 is imagebmff.cpp and is higher priority than this.

@clanmills
Copy link
Collaborator

I've got good and ungood news about this.

Good News

I know what's wrong with this. The tag 0x2010 is not being cipher correctly. It's being double deciphered and the fix correlates perfectly with code required in #582 #935 to invoke sonyTagEncipher.

Additionally, as suspected, the patch also fixes #1472.

The patch is:

674 rmills@rmillsmm-local:~/gnu/github/exiv2/0.27-maintenance/build $ git diff
diff --git a/src/tiffcomposite_int.cpp b/src/tiffcomposite_int.cpp
index a6bf925b..e530c169 100644
--- a/src/tiffcomposite_int.cpp
+++ b/src/tiffcomposite_int.cpp
@@ -30,6 +30,7 @@
 #include "tiffcomposite_int.hpp"
 #include "tiffvisitor_int.hpp"
 #include "makernote_int.hpp"
+#include "sonymn_int.hpp"
 #include "value.hpp"
 #include "error.hpp"
 #include "enforce.hpp"
@@ -1433,6 +1434,19 @@ namespace Exiv2 {
             uint16_t lastTag = static_cast<uint16_t>(lastDef->idx_ / cfg()->tagStep());
             idx += fillGap(mioWrapper, idx, lastDef->idx_ + lastDef->size(lastTag, cfg()->group_));
         }
+#if 1
+        if (cfg()->cryptFct_) {
+            // Re-encrypt buffer if necessary
+            CryptFct cryptFct = cfg()->cryptFct_;
+            if ( cryptFct == sonyTagDecipher ) {
+                 cryptFct  = sonyTagEncipher;
+            }
+            DataBuf buf = cryptFct(tag(), mio.mmap(), static_cast<uint32_t>(mio.size()), pRoot_);
+            ioWrapper.write(buf.pData_, buf.size_);
+        } else {
+            ioWrapper.write(mio.mmap(), static_cast<uint32_t>(mio.size()));
+        }
+#else
         DataBuf buf;
         if (cfg()->cryptFct_) {
             buf = cfg()->cryptFct_(tag(), mio.mmap(), static_cast<uint32_t>(mio.size()), pRoot_);
@@ -1443,7 +1457,7 @@ namespace Exiv2 {
         else {
             ioWrapper.write(mio.mmap(), static_cast<uint32_t>(mio.size()));
         }
-
+#endif
         return idx;
     } // TiffBinaryArray::doWrite

(The patch includes the old code. The old code will be totally removed in the PR).

Ungood News

There are lots of failures in the test suite coming from the Nikon tags. Curious because they are encrypted and this change involved the encryption.

I will investigate the exceptions in the test suite before preparing the PR for this and #1472.

@clanmills
Copy link
Collaborator

I fix the Nikon/test exceptions with a tiny change to the patch above.

I've submitted a PR #1476. The changes to the test suite are for #1471 and #1472. All existing tests are working without change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants