Project

General

Profile

Bug #1147

writeMetadata overwrites files without the +w bit

Added by Dan Fandrich almost 6 years ago. Updated almost 6 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Robin Mills
Category:
basicio
Target version:
0.26
Start date:
01 Jan 2016
Due date:
% Done:

100%

Estimated time:
5.00 h

Description

Exiv2::JpegBase::writeMetadata will overwrite a read-only file instead of returning an error message. If ro.jpg is a read only JPEG file (e.g. mode 444) with EXIF tags, the attached program will show the error message:

ro.jpg: Failed to open file (w+b): Permission denied (errno = 13)

in ver. 0.24 (as expected) but will silently overwrite the file in ver. 0.25 (unexpected and dangerous).

Files

ro.cpp (374 Bytes) ro.cpp Dan Fandrich, 01 Jan 2016 20:56
musl-ro-write.c (247 Bytes) musl-ro-write.c Dan Fandrich, 02 Jan 2016 08:07

Associated revisions

Revision 4159 (diff)
Added by Robin Mills almost 6 years ago

#1147 Report uid, euid and gid in exiv2 --verbose --version

Revision 4160 (diff)
Added by Robin Mills almost 6 years ago

#1147. Correction to r4159 for linux build breaker (linux doesn't have include uuid/uuid.h)

Revision 4161 (diff)
Added by Robin Mills almost 6 years ago

#1147. Correction for Cygwin/MinGW build-breaker in r4159. Don't compile the uid/euid/gid code on any windows build.

Revision 4162 (diff)
Added by Robin Mills almost 6 years ago

#1147. Fixed a typo.

Revision 4163 (diff)
Added by Robin Mills almost 6 years ago

#1147 Fixed travis build-breaker.

History

#1

Updated by Robin Mills almost 6 years ago

  • Assignee set to Robin Mills
  • Target version set to 0.26

Interesting find. I instantly wonder if Memory Mapping is the culprit. Perhaps he knows nothing about the UNIX file system permission bits. I'll investigate tomorrow. If you want to help, you could:

1 unset MM in exv_conf.h and rebuild.
2 identify the platforms involved.

Happy New Year.

#2

Updated by Dan Fandrich almost 6 years ago

Sorry, I forgot to mention it before: this was on Linux 4.1.13 x86_64 with MUSL 1.1. It occurs to me now that the working case with exiv2 0.24 was built against glibc 2.20, so the libc difference might actually be an issue rather than the exiv2 version. I'll try building 0.25 against glibc when I get a chance.

#3

Updated by Dan Fandrich almost 6 years ago

I was able to quickly build the test program against exiv2 0.25 with glibc 2.20, and it works correctly (Permission denied). So, the problem looks related to MUSL or exiv2's use of it.

#4

Updated by Robin Mills almost 6 years ago

  • Category set to basicio
  • Status changed from New to Assigned
  • % Done changed from 0 to 30
  • Estimated time set to 3.00 h

I'd never heard of MUSL until now. Ah, there's always something to discover and learn in Software.

The trunk seems to be OK on both Mac (10.11.2) and Linux (Kubuntu 15.04)

$ uname -a
Linux rmillsmbp-k1504 4.2.0-22-generic #27-Ubuntu SMP Thu Dec 17 22:57:08 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux
$ cp /media/psf/Home/temp/Stonehenge.jpg .
$ sudo chmod 0000 Stonehenge.jpg 
$ ls -alt Stonehenge.jpg 
---------- 1 rmills rmills 6757827 Jan  1 22:49 Stonehenge.jpg
$ exiv2 -pa --grep Comment/i Stonehenge.jpg 
Exiv2 exception in print action for file Stonehenge.jpg:
Stonehenge.jpg: Failed to open the data source: Permission denied (errno = 13)
$ chmod +r Stonehenge.jpg 
$ exiv2 -pa --grep Comment/i Stonehenge.jpg 
Exif.Photo.UserComment                       Undefined  44                                      
$ ls -alt Stonehenge.jpg 
-r--r--r-- 1 rmills rmills 6757827 Jan  1 22:49 Stonehenge.jpg
$ exiv2 -M"set Exif.Photo.UserComment Happy New Year" Stonehenge.jpg 
Exiv2 exception in modify action for file Stonehenge.jpg:
Stonehenge.jpg: Failed to open file (a+b): Permission denied (errno = 13)
$ chmod 0644 Stonehenge.jpg 
$ exiv2 -M"set Exif.Photo.UserComment Happy New Year" Stonehenge.jpg 
$ exiv2 -pa --grep Comment/i Stonehenge.jpg 
Exif.Photo.UserComment                       Undefined  22  Happy New Year
$ 
$ 
$ 
$ cp /Home/Downloads/ro.cpp .
$ g++ ro.cpp -lexiv2 -o ro
$ ls -alt Stonehenge.jpg 
-rw-r--r-- 1 rmills rmills 6757827 Jan  1 22:51 Stonehenge.jpg
$ chmod 0400 Stonehenge.jpg 
$ ls -alt Stonehenge.jpg 
-r-------- 1 rmills rmills 6757827 Jan  1 22:51 Stonehenge.jpg
$ cp Stonehenge.jpg ro.jpg
$ ls -alt ro.jpg
-r-------- 1 rmills rmills 6757827 Jan  1 23:05 ro.jpg
$ ./ro
ro.jpg: Failed to open file (a+b): Permission denied (errno = 13)
$
Can you help me pin down the exact configuration required to reproduce this. Please add the following couple of lines to your code. This will dump a load of information both about the build and the libraries which have been loaded at run-time:
    exv_grep_keys_t keys;
    Exiv2::dumpLibraryInfo(std::cout,keys);
Incidentally, these two lines of code can be executed by the command: 
 $ exiv2 --verbose --version
However please add the code to ro.cpp because it will list the run-time libraries in use by ro(.exe) which may be different from those loaded by exiv2(.exe). If exiv2(.exe) is OK, and ro(.exe) is not safe, it has to be something at runtime and you can see the libraries actually in use.

If you don't find an answer, send me output from your modified ro.cpp and I'll have a look at it in the morning. Very optimistic that we'll resolve this tomorrow.

#5

Updated by Dan Fandrich almost 6 years ago

This is the output:

exiv2=0.25.0
platform=linux
compiler=G++
bits=64
dll=1
debug=0
version=5.2.0
date=Nov 10 2015
time=15:38:37
svn=0
ssh=0
curl=0
id=$Id: version.cpp 3800 2015-05-08 22:26:36Z robinwmills $
executable=/root/ro
library=/usr/lib/libstdc++.so.6
library=/usr/lib/libgcc_s.so.1
library=/lib/ld-musl-x86_64.so.1
library=
library=/lib/libz.so.1
library=/usr/lib/libexpat.so.1
have_regex=1
have_strerror_r=1
have_gmtime_r=1
have_inttypes=1
have_libintl=0
have_lensdata=1
have_iconv=1
have_memory=1
have_memset=1
have_lstat=1
have_stdbool=1
have_stdint=1
have_stdlib=1
have_strlib=0
have_strchr=1
have_strerror=1
have_strerror_r=1
have_strings_h=0
have_strtol=1
have_mmap=1
have_munmap=1
have_sys_stat=1
have_timegm=1
have_unistd_h=0
have_sys_mman=1
have_libz=0
have_xmptoolkit=1
have_bool=1
have_strings=1
have_sys_types=1
have_unistd=1
have_unicode_path=0
enable_video=0
enable_webready=0

Running 'exiv2 -M"set Exif.Photo.UserComment Happy New Year" ro.jpg' results in no error and the ro.jpg file gaining a new EXIF tag.

#6

Updated by Dan Fandrich almost 6 years ago

I built exiv2 0.25 from source and commented the mmap/munmap lines from exv_conf.h, but it still failed. Since the problem shows up with the exiv2 command-line I'm using that to test now:

exiv2=0.25.0
platform=linux
compiler=G++
bits=64
dll=1
debug=0
version=5.3.0
date=Jan 2 2016
time=07:22:43
svn=0
ssh=0
curl=0
id=$Id: version.cpp 3800 2015-05-08 22:26:36Z robinwmills $
executable=/root/exiv2-0.25/install/usr/local/bin/exiv2
library=/usr/lib/libstdc++.so.6
library=/usr/lib/libgcc_s.so.1
library=/lib/ld-musl-x86_64.so.1
library=
library=/lib/libz.so.1
library=/usr/lib/libexpat.so.1
have_regex=1
have_strerror_r=1
have_gmtime_r=1
have_inttypes=1
have_libintl=0
have_lensdata=1
have_iconv=1
have_memory=1
have_memset=1
have_lstat=1
have_stdbool=1
have_stdint=1
have_stdlib=1
have_strlib=0
have_strchr=1
have_strerror=1
have_strerror_r=1
have_strings_h=0
have_strtol=1
have_mmap=0
have_munmap=0
have_sys_stat=1
have_timegm=1
have_unistd_h=0
have_sys_mman=1
have_libz=0
have_xmptoolkit=1
have_bool=1
have_strings=1
have_sys_types=1
have_unistd=1
have_unicode_path=0
enable_video=0
enable_webready=0

#7

Updated by Dan Fandrich almost 6 years ago

I did some more tests and discovered the attached program fails with MUSL as well, so it looks like a pure MUSL issue.

#8

Updated by Dan Fandrich almost 6 years ago

Now I'm embarrassed. My MUSL tests were all run in a container and as the root user, and I've just now discovered that root can write to files even though they don't have the write bit set. This was news to me. Sorry about the noise.

#9

Updated by Robin Mills almost 6 years ago

  • Status changed from Assigned to Closed
  • % Done changed from 30 to 100

Well done, Dan. I wake up and read this and you've solved it. Good Job. Thanks for working hard and getting to the bottom of this.

#10

Updated by Robin Mills almost 6 years ago

r4159 Added code to src/version.cpp to report uid/euid/gid:

781 rmills@rmillsmbp:~/gnu/exiv2/trunk $ echo "I am " $(whoami) ; id ; exiv2 -vVg id
I am  rmills
uid=501(rmills) gid=20(staff) groups=20(staff),402(com.apple.sharepoint.group.1),701...
...
uid=501
euid=501
gid=20
...
$ sudo bash -c 'echo "I am " $(whoami) ; id ; exiv2 -vVg id'
I am  root
uid=0(root) gid=0(wheel) groups=0(wheel),403(com.apple.sharepoint.group.2),402(com.apple.sharepoint.group.1),...
...
uid=0
euid=0
gid=0
$

#11

Updated by Thomas Beutlich almost 6 years ago

Robin,

Happy New Year to you, your wife and all the best for your spare-time projects.

In r4162 you changed the _WIN32 macro to WIN32, however _WIN32 is correct for the Windows OS. I use it this way for all my C/C++ projects. See https://sourceforge.net/p/predef/wiki/OperatingSystems/ -> Windows

Thomas

#12

Updated by Robin Mills almost 6 years ago

Yes. Happy New Year to you.

This is a good observation. I used _WIN32 when I added the code on the Mac. It broke the MSVC build and I changed it to WIN32 and that was fine. I'm not sure that I define either WIN32 or _WIN32, I think it might appear in the code from the platform. So, I haven't changed _WIN32, I replaced the use of the undefined _WIN32 with WIN32 because it worked!

Are you suggesting that I review and modify every reference to WIN32 with _WIN32 and make sure it is defined?

#13

Updated by Robin Mills almost 6 years ago

  • Status changed from Closed to Resolved
  • % Done changed from 100 to 90
  • Estimated time changed from 3.00 h to 5.00 h

I'm reopening this to continue the discussion with Thomas.

#14

Updated by Robin Mills almost 6 years ago

  • Status changed from Resolved to Closed
  • % Done changed from 90 to 100

As Thomas hasn't bothered to answer my question and there is no bug here, this matter is being closed again.

Thomas: If you want something done about this, please deal with it as I am busy and have about 500 hours of work planned for v0.26. http://dev.exiv2.org/projects/exiv2/news

I suspect that changing this will consume several hours. Of course this isn't difficult - however there will be unexpected consequences which take time to investigate and resolve.

Also available in: Atom PDF