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

SECURITY ISSUE: Signal doesn't strip EXIF from sent images, including precise location data #1984

Closed
1 task done
no-usernames-left opened this issue Apr 14, 2017 · 58 comments
Labels

Comments

@no-usernames-left
Copy link

  • I have searched open and closed issues for duplicates

Bug description

Signal for iOS includes EXIF data in sent images, including precise GPS data if present. Instead, EXIF data should be stripped (possibly except for the absolute bare minimum such as dimension and rotation).

Steps to reproduce

  • Enable Location access for Camera.app
  • Take a picture with Camera.app
  • Send that picture from the Camera Roll using Signal
  • Recipient uses the share sheet to save the image, then opens it in an app such as ExifWizPro

Actual result: Recipient sees the precise location the image was taken, along with all other EXIF fields
Expected result: Recipient sees no EXIF data whatsoever, as is the case when using Threema

Device info

Device: iPhone 6
iOS version: 10.3.1
Signal version: 2.10.1

@ebelinski
Copy link

I'm not so sure about the necessity of this. If I wanted to remove EXIF data, I could get an app that does it, and send the stripped version. Otherwise I would want to send the photo with EXIF data, if I want my recipient to know where and when it was taken, and the aperture, shutter speed, etc. Maybe it could be optional in the app?

On a side note if we want Signal to be sending completely "anonymous photos", we should definitely be considering the issue of camera fingerprinting as well. Without making special modifications to a photo, the subtle imperfections of a photo can be used to identify the camera that took it. For example, if you use your phone's camera to take pictures that you post publicly on Facebook, and send a different photo from the same camera to a person anonymously, camera fingerprinting based on the contents of the actual photo could be used to tie the anonymous photo to your Facebook identity, without checking EXIF data.

@theochino
Copy link

theochino commented Apr 16, 2017 via email

@big-r81
Copy link
Contributor

big-r81 commented Apr 17, 2017

My thoughts:

I don't think we should go this way.

Why doesn't you forbid the camera app to use the location services if security matters? Why not disable location and/or other services at system level?

The problem is, that some users want a feature other users not and if every feature will get an on/off switch the preference pane will grow with many switches. Maybe they depend on each other or are mutually exclusive... NO that isn't user fiendly and maybe results in deceptive security...

@sigenc
Copy link

sigenc commented Apr 18, 2017

I think the android version strips the exif data. Maybe we should change it for consistency.

Edit:

I was wrong. It is incidentally stripped because of image scaling:

signalapp/Signal-Android#5968

@no-usernames-left
Copy link
Author

The steps to reproduce indeed included enabling Location Services for Camera.app, but the threat exists in many situations, including passing along an image received from someone else. (For example, John McAfee's location while in hiding was revealed precisely this way.)

Information leakage via metadata is a thing, and pretending it isn't only undermines confidence in this project.

@theochino
Copy link

theochino commented Apr 18, 2017 via email

@strugee
Copy link

strugee commented Apr 18, 2017

including passing along an image received from someone else.

I'm unclear on how the onus is now on the "middleman" in this situation. If the receiver doesn't want their location shared, shouldn't they be the ones turning off location metadata?

@ebelinski
Copy link

ebelinski commented Apr 18, 2017

If the receiver doesn't want their location shared, shouldn't they be the ones turning off location metadata?

I guess I can see the case for stripping EXIF from a user friendly perspective. The average Signal user may not be aware that sending a picture means sending the location the picture was taken if that feature is turned on, and on iPhone it usually is. And even if a user knows that the feature is turned on to collect location information for each photo, it's totally possible that he wouldn't be aware that the location data is stored inside the image file itself, and that Signal sends the whole file. After all, iOS abstracts away the concept of files, so many users don't know how metadata is stored on iOS.

Edit: I'm also reminded of the story of Higinio O. Ochoa III, a hacker that got caught because he used software to remove the EXIF data from an image he wanted to upload, but then accidentally uploaded the original. Even when you know about EXIF, you could still screw up.

@kaktusztea
Copy link

I would suggest a global checkbox in Settings (Strip image metadata).
Default: ON

@strugee
Copy link

strugee commented Apr 24, 2017

The answer is not more options: https://github.com/WhisperSystems/Signal-Android/blob/master/CONTRIBUTING.md#development-ideology

@theochino
Copy link

theochino commented Apr 24, 2017 via email

@ephemer
Copy link

ephemer commented Apr 26, 2017

If users really want to send the photo's location they can just tell the recipient where the photo was taken (or share a pin from Photos.app after sending it).

In my opinion, most "normal" users^ won't:

  1. Save the photo to Photos.app or elsewhere at all
  2. Know that location data is available in its metadata if they do

... meaning that not stripping EXIF metadata is at best helping only a minimal number of users (for whom simple workarounds exist) and at worst jeopardising people's security without their knowledge.

If the answer is not adding settings (and I agree that it is not), then I'd suggest the answer is strongly in favour of stripping the EXIF data.


^ the development guidelines also state that there is no such thing as a power user, and providing options with them in mind is often especially damaging.

@kaktusztea
Copy link

After read ephemer's comment, an idea:

  • default behaviour: strip EXIF data
  • long press (1 or 2 sec) on Photo attach label (in dropdown list): do not strip

So "do not strip" would be a deliberate act in a rare, but real scenario.

@ebelinski
Copy link

Some metadata I think we should definitely not strip by default. For example, when you edit a picture and rotate it with the iOS Photos app, then you delete all metadata, the orientation goes back to the original, because the photo itself wasn't changed, just some metadata.

On an unrelated note, has anyone else noticed that this issue's number is #1984? 🤔😄

@megraf
Copy link

megraf commented May 2, 2017

I agree that stripping should be OPT-OUT rather than OPT-IN. Chances are users won't notice if their EXIF is striped, but they should have the option either way. Signal's initial privacy orientation should make it pretty clear that EXIF should be stripped IMHO.

@caveman1973
Copy link

Signal should remove ALL EXIF data, not only location.

If you want to send a picture with EXIF data, use another app.

@ghost
Copy link

ghost commented Jul 20, 2017

Signal absolutely should offer to remove metadata. Period. Adding settings is the answer. Firefox has many, many options in "about:config". So Signal should make the best choices for privacy but allow people to dig through a more granular menu somewhere. It's out of the way unless you go looking through the settings. That way people who actually care will find it, but people who don't care are protected by strong default settings.

This isn't going to clutter the UI. It's very simple and will help protect the privacy of most people using this app.

You have to realize that we are in an extremely small sample size. Most people have no idea what can be leaked through metadata. Most people don't understand the details of surveillance and software to the degree that you guys do.

How many hundreds of thousands of people do you think use this app? Most of them will probably never even visit this repository, let alone understand the dangers of metadata.

So if Signal can do it, it's worth helping out people who aren't as privacy conscious as we are.

@tomrittervg
Copy link

I disagree. Signal should not offer to remove the metadata. The word 'offer' implies it might be a prompt. Signal should strip (at a minimum) geolocation data - period.

EXIF data in photos is a fidelity signal from a camera to a photo. It preserves as much metadata about the image as possible for future categorization and record keeping. It's similar to saving a photo in RAW format.

Signal is not designed to be a photo archiving app or editing or storage app. It's for sharing messages and photos. There is no mechanism in Signal to view EXIF data, and doing so is something only particularly technical people would think of or even be able to try.

And geolocation data is extremely sensitive (it's extremely common to take photos in your own home) and in many or most cases one does not want to share their home address with all the individuals one may wish to send a picture to.

Signal to do the 'safe' thing by default, which would be at a minimum to strip geolocation data. Maybe, maybe there could be an option to disable this feature, buried somewhere - but I don't even think the option is necessary.

@ghost
Copy link

ghost commented Jul 25, 2017

Okay I agree. Sorry for not being precise with my words. Essentially I meant that Signal should have the capability to do this. And it would be accessible to manage in settings.

Safe by default is the right way to handle. Thanks!

@NSDestr0yer
Copy link

NSDestr0yer commented Aug 1, 2017

Another angle of looking at this, is that because sometimes it strips EXIF data due to resizing (Android), people might gather that it does all the time. Ex. I do a simple test to make sure it's safe, it passes, then I have a false sense of security and send pictures without thinking about it again.

For the iOS version, the else case for selecting images from the library (didFinishPickingMediaWithInfo) uses PHAsset which includes metadata, the other cases use UIImage which does not contain metadata information because UIImage represents a decoded image that discards metadata.

@Calefornia I agree :) - I've had a few people now send me pictures from Android and I save them on iOS camera roll. Then later I look at my Photo's app and that picture shows up in the "Places" group. Turned out I can zoom right to where their house was!! In all cases, the user didn't know about this and I had to give them instructions on how to disable location services.

@ghost
Copy link

ghost commented Dec 16, 2017

Here's a link to the Metadata Anonymisation Toolkit, which in turn provides links to similar projects:

https://mat.boum.org/

Hopefully someone can integrate code or modify it as needed for Signal.

@NSDestr0yer
Copy link

For iOS, the current logic is that in order to avoid re-encoding GIF/PNGs as JPEG we are retrieving the raw data using the Photo Framework (PHAsset instead of using UIImagePickerControllerOriginalImage), which happens to include EXIF and GPS data with it.

So, in order to keep the current functionality and code as is, one option available would be to add an extra step - after retrieving that NSData from the Photo Framework,
remove the kCGImagePropertyExifDictionary and kCGImagePropertyGPSDictionary from it. This will remove all the EXIF and GPS data before sending the image. Because the data returned from the Photo Framework is immutable, a copy of the data with the stripped attributes would have to be made.

This is something I could implement and do a PR for, unless anyone has any objections to that solution or better suggestions here.

@michaelkirk
Copy link
Contributor

We'd consider a PR which strips out non-orientation exif metadata by default, with an option to leave it in (settings > privacy > remove image meta data).

Be aware that the image picker is not the only source of images. Additionally images can come from the document picker (e.g. iCloud), copy/paste, etc.

We'd want the same behavior regardless of source. My feeling is that the SignalAttachment class, which all sources flow through, may be the right place to implement this.

@NSDestr0yer
Copy link

Okay, I can work on and implement this in SignalAttachment and then do a PR for it.

@mailinglists35
Copy link

mailinglists35 commented Feb 21, 2018

so much nonsense talk for something you can do it yourself (free app Koredoko can strip exif https://itunes.apple.com/us/app/koredoko-exif-and-gps-viewer/id286765236?mt=8)

meanwhile we don't even have yet a way to cleanup received media stored locally, but you all keep complaining for something totally unrelevant

@cbanowsky
Copy link

This is a must. The aim is to constantly improve and make the software better and protect privacy. Signal is used by people who dont understand the types of protection it truly offers. Knowing that these users dont know what EXIF data is I think it is essential that a user be able to send photos without revealing their private information without their understanding of said EXIF data. I did the same yesterday. Geolocated a person to their precise real time location and disclosed to said individual to ensure their apps do not permit geolocation information stored in photos.

But I feel as the filer of this issue feels, that that for the majority this is a security issue.

@cbanowsky
Copy link

In regards to camera fingerprinting. The process isnt as easy as exiftool.

@allanlw
Copy link

allanlw commented Apr 17, 2018

Does #2907 getting merged mean this issue can be closed?

@timfallmk
Copy link

@databonanza Please read the discussion before commenting. Scrolling up will show you it has already been done, it just needs to be closed.

@stale
Copy link

stale bot commented Jan 23, 2022

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the wontfix label Jan 23, 2022
@ckujau
Copy link

ckujau commented Jan 23, 2022

While the original report was about "Signal doesn't strip [enough] EXIF data", it turned out that people disagreed on that point and maybe a toggle was needed to strip / don't strip meta data from images (and other media), see also #5968 Image Meta Data Removal for a similar discussion for the Android version. So far this toggle has not been implemented for either platform, so I think this report is still valid.

There was an argument about not wanting too many configuration items in the application, but we do have "Sealed Sender" and "Advanced PIN" settings now, so I guess this point is moot now :-)

@stale stale bot removed the wontfix label Jan 23, 2022
@stale
Copy link

stale bot commented May 21, 2022

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the wontfix label May 21, 2022
@ckujau
Copy link

ckujau commented May 21, 2022

Is there a way to prevent stale bot from trying to close this issue every 5 months?

@stale stale bot removed the wontfix label May 21, 2022
@hex-m
Copy link

hex-m commented May 21, 2022

@ckujau This project uses the default configuration of the stale bot. You can read more about it in the README of the bot.

@stale
Copy link

stale bot commented Aug 31, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Aug 31, 2022
@databonanza
Copy link

Stale devs not stale thread.

This issue is quite important. Let's ban stale bot from certain threads that are important. Also, this should be an easy fix.

@NiklasBr
Copy link

Why won't the bot remove the label?

@stale
Copy link

stale bot commented Sep 7, 2022

This issue has been closed due to inactivity.

@stale stale bot closed this as completed Sep 7, 2022
@banool
Copy link

banool commented Sep 7, 2022

5 years of dev silence on a hot issue for it to just to get closed by stale bot, time to switch messaging apps.

@databonanza
Copy link

5 years of dev silence on a hot issue for it to just to get closed by stale bot, time to switch messaging apps.

To what though? I can't believe this is just left alone. I have tried to pull exif data from signal photos and "there is none" but I don't know if it's because Signal is scrubbing it. This is a big problem, who is the person we need to @ to get their attention on this?

@kaktusztea
Copy link

As far as I know Signal DOES strip the EXIF data for a few years now.
Some folks above wanted to extend this ticket with some feature requests which are not part of this issue IMHO.
So I think closing this ticket is justified. (let's downvote me).

@banool
Copy link

banool commented Sep 8, 2022

If it has actually been addressed it should be explicitly closed, not closed by stalebot.

@m-graf
Copy link

m-graf commented Sep 8, 2022

If it has actually been addressed it should be explicitly closed, not closed by stalebot.

This. Can someone please mark this as resolved (or the equivalent?)

@kaktusztea
Copy link

If it has actually been addressed it should be explicitly closed, not closed by stalebot.

Actually, that is right, but devs doesn't really review on old tickets unfotunatelly 🙁
There are a plenty of tickets which could be already closed immediately if devs would do a full overview on open tickets.

@tomrittervg
Copy link

I don't see any indication that the app actually strips EXIF data. If it does, then someone can link to the functionality.

@kaktusztea
Copy link

I don't see any indication that the app actually strips EXIF data. If it does, then someone can link to the functionality.

I think this is it:
9adf79c

@no-usernames-left
Copy link
Author

If it has actually been addressed it should be explicitly closed, not closed by stalebot.

Is it actually resolved? Moxie still won't let us save our data on iOS since I opened this issue five fucking years ago, so Signal is now my third choice behind Threema and Matrix.

Shrug.

@tomrittervg
Copy link

I don't see any indication that the app actually strips EXIF data. If it does, then someone can link to the functionality.

I think this is it: 9adf79c

Good hunting. That commit removes the option to remove metadata so it's done by default. I eventually found this code in the master branch.

I found this comment on Android and this for desktop.

Not well described...

@no-usernames-left
Copy link
Author

I found this comment on Android and this for desktop.

What about iOS?

@no-usernames-left
Copy link
Author

no-usernames-left commented Sep 8, 2022

/**
   * Compresses the images. Given that we compress every image, this has the fun side effect of
   * stripping all EXIF data.
   */

Why is it presumed that EXIF is stripped due to compression? If that's true, all images would lose their rotation, and that isn't the case.

Sus.

@databonanza
Copy link

If it has actually been addressed it should be explicitly closed, not closed by stalebot.

Actually, that is right, but devs doesn't really review on old tickets unfotunatelly 🙁 There are a plenty of tickets which could be already closed immediately if devs would do a full overview on open tickets.

There is no need for a full review. There is a need for accountability in a open source project. When you fix things that are related to "issues" you update the issue with the current status.

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

Successfully merging a pull request may close this issue.