16 ms·

Hi there, Signal-Android developer here. I updated the issue to reflect this, but this bug has been fixed. I was tracking it on a separate issue, and had forgot

Hi there, Signal-Android developer here. I updated the issue to reflect this, but this bug has been fixed. I was tracking it on a separate issue, and had forgotten to close this one. We do, in fact, take issues like this very seriously. This bug was extraordinarily rare, and because we have no metrics/remote log collection, there was an initial period where we had to spend time adding logging and collecting user-submitted logs to try to track it down. As soon as we were able to pick up a scent, it was all we worked on, and we were able to get a fix out very quickly.

TekMol 5y ago
Can you provide a link to the commit that fixes it? Shouldn't there have been an announcement to inform users what has been leaked and under which circumstances? How can user A send an image to user B that neither of them took? Isn't everything end-2-end encrypted? Then how can unencrypted data from user C end up on the device of user B?
- agilob 5y ago
  No PR with name that would suggest the fix in the client https://github.com/signalapp/Signal-Android/pulls?q=is%3Apr+author%3Agreyson-signal+is%3Aclosed https://github.com/signalapp/Signal-Android/pulls?q=is%3Apr+... and no PR from OP in the opensource part of the server https://github.com/signalapp/Signal-Server/pulls?q=is%3Apr+is%3Aclosed+author%3Agreyson-signal https://github.com/signalapp/Signal-Server/pulls?q=is%3Apr+i...
  - deleted 5y ago
    [deleted]
  - seg_lol 5y ago
    *edit, I think this issue was specific to the Android client, the desktop client has a totally different sqlite schema. The child comment to your comment is deleted, but I think autoincrement IDs shouldn't be used under an ambient authority context. It would make more sense to have IDs based on an LSF or Feistel sequence, perhaps split into a master ID and a conversation sequence. Autoincrement on this field makes it easy for off by one errors. Even just moving to guids and maintaining a proper parent child relationship would have prevented this. Or maybe there should be a database per conversation (set of all parties). https://github.com/signalapp/Signal-Android/commit/83086a5a2b8cae3ac40dea75d8c9533457a31858 https://github.com/signalapp/Signal-Android/commit/83086a5a2... https://github.com/signalapp/Signal-Android/commit/b9657208fea1c7bcfb90b7400037a7858ba56516 https://github.com/signalapp/Signal-Android/commit/b9657208f... Row IDs shouldn't have so much power.
    - WesolyKubeczek 5y ago
      The good thing about autoincrementing unsigned 64-bit integers is that 1) it's insanely fast, 2) SQLite is doing it automatically, 3) seriously, why don't they have it on all tables yet? SQLite guarantees no collisions within a table. Doing homegrown ID generation is how such bugs are being introduced in the first place. Your application level trigger got bypassed, oops. Your check was experimentally disabled and left like that for a year until people started noticing, oops. If you make an SQLite-backed application and it has a bug like this, I can safely bet $500 it's not going to be SQLite that has the bug. Just don't expose the numeric IDs in links, and generate your UUIDs in tables where you need to point to rows in an outside-accessible link. This is database 101, for crying out loud.
- tomudding 5y ago
  > Can you provide a link to the commit that fixes it? If I understand the issue [0] correctly, these two commits should be the fix: https://github.com/signalapp/Signal-Android/commit/e90fa05d608d49759e4cbac8cf233a797e0ee395 https://github.com/signalapp/Signal-Android/commit/e90fa05d6... https://github.com/signalapp/Signal-Android/commit/b9657208fea1c7bcfb90b7400037a7858ba56516 https://github.com/signalapp/Signal-Android/commit/b9657208f... The former updates how recipients (or really threads, I suppose) are merged (the issue occurred when trimming threads) and the latter changes the way how thread ids are generated (now automatically incremented). Together they should prevent unrelated recipients (threads) from being merged. [0]: https://github.com/signalapp/Signal-Android/issues/10247#issuecomment-886239978 https://github.com/signalapp/Signal-Android/issues/10247#iss...
  - deleted 5y ago
    [deleted]
  - winrid 5y ago
    I love the terrible commit name. "Updating recipient merging."
  - hackinthebochs 5y ago
    Using incrementing ids as your source of ownership is just asking for trouble. This just means a programming error can have a high probability of ids lining up and leaking resources. Guids make this practically impossible.
    - rattray 5y ago
      Interesting, I hadn't thought of that advantage of uuids/guids before.
    - fulafel 5y ago
      I wonder if there's any widely implemented programming pattern that would catch this better, eg consisting of concatenated type code and id. Using GUIDs here would still hide the bug, not flag an error.
- Popegaf 5y ago
  From the explanation [0], this looks like the commit [1]. I do have to say though, the commit messages are very barren and barely any reference an issue directly. My guess is that they develop on private branches with an internal issue tracker and decide not to reference issues as that would be confusing. It would help a bunch if they referenced the public issues and made it clear which issues they're working on. They don't seem to use Github Projects [2]. All in all, the communication / transparency of the project is lacking - even though I'm glad they're providing something usable. Hopefully Matrix will be able to provide something as easily usable as well. [0]: https://github.com/signalapp/Signal-Android/issues/10247#issuecomment-886239978 https://github.com/signalapp/Signal-Android/issues/10247#iss... [1]: https://github.com/signalapp/Signal-Android/commit/83086a5a2b8cae3ac40dea75d8c9533457a31858 https://github.com/signalapp/Signal-Android/commit/83086a5a2... [2]: https://github.com/signalapp/Signal-Android/projects https://github.com/signalapp/Signal-Android/projects
- konschubert 5y ago
  If it’s a client bug that just switches out recipients, then the messages will just get encrypted for the wrong recipient.
lwhi 5y ago
I appreciate that this was a difficult and rare bug, but for an app that sells itself as 'secure', it feels like this isn't acceptable. How can users be assured that this type of issue won't occur again?
- nerbert 5y ago
  ‘Many forms of secure messaging systems have been tried, and will be tried in this world of sin and woe. No one pretends that Signal is perfect or all-wise. Indeed it has been said that Signal is the worst messaging except for all those other forms that have been developed from time to time…’ Winston S Churchill, 11 November 1947
  - geofft 5y ago
    Sure, but there's a small theoretical difference with democracy. You have to live under some system of government. You don't have to use a secure messenger. You can choose to have sensitive conversations in person or not have them at all. I agree that in practice, a lot of people are going to use their phones for relatively sensitive conversations, and in practice, Signal remains the best choice for doing so. But there are a few real threat models where the options aren't Signal vs. SMS / Google Chat / Discord / etc., the options are Signal vs. nothing. For instance, you could be a journalist deciding whether to ask clarifying questions to a government whistleblower via Signal or meet up with them in a park. You could be an activist/demonstrator under a repressive regime deciding whether to coordinate some action this weekend via Signal or hold off on it entirely and tactically preserve your freedom. And so forth. For those people, if (and to be clear this is a big "if," while this issue is one serious piece of evidence it is nonetheless inconclusive) Signal isn't trustworthy, it doesn't matter if Signal is the least-bad of the options. (Also, it's not like Signal is the only e2e messenger around. There's iMessage/FaceTime, for instance. Churchill's claim was that the abstract idea of democracy was good, not that any concrete implementation like the British government was good.)
    - helmholtz 5y ago
      You make some fair points. But even from the eyes of those people, what is the alternative? Is iMessage guaranteed to not have any hidden exploits out there? And on the flip side, what do they lose out on by only having those conversations in person? Well, I'd argue that their world becomes a lot smaller, and there sources are instantly at a higher risk.
andrewguenther 5y ago
> This bug was extraordinarily rare, and because we have no metrics/remote log collection, there was an initial period where we had to spend time adding logging and collecting user-submitted logs to try to track it down. Without telemetry, can you actually back up the claim that this issue was extremely rare?
- hnarn 5y ago
  Some details on how this assumption was made would be nice, but I think it's pretty obvious that any developer involved in a project can make a reasonable assumption of how rare a bug is depending on the technical details on what is required for the bug to happen. For example, if we say for the sake of argument that a hypothetical bug requires you to have more than ten contacts of the exact same name and these also need to share the same country and area code, one can make the assumption this use case is very rare without knowing the exact number of users that this applies to, just based on common sense regarding how the application is normally used. edit: The linked github issue says: > The TL;DR is that if someone had conversation trimming on, it could create a rare situation where a database ID was re-used in a way that could result in this behavior. It was very difficult to track down, with earlier phases involving getting additional logging into builds. Once we had some more information, it did in fact become our top priority, a fix was made, and we got it out as quickly and as safely as possible. The fix itself should make it so that database issues like the one that caused this bug can't happen again.
  - NullPrefix 5y ago
    >for the sake of argument that a hypothetical bug requires you to have more than ten contacts of the exact same name and these also need to share the same country and area code This is only rare if you have a small social circle. My circle has multiple first name-collisions of at least 5 participants, but my circle is not very big and the area code is also quite small. Some countries do not use area codes for mobile phone numbers, which are used for Signal, meaning country code is the only area limiting factor.
    - 5y ago
spicybright 5y ago
Just wanted to say, thank you so much for your contributions. Signal is an amazing product, and things like these happen to the best of projects.
nathan_phoenix 5y ago
> we were able to get a fix out very quickly. I'm not sure if 8 months can be categorized as fast... The issue was posted on Dec 4, 2020, and the fix (5.17) was released on July 21. Also, sounds like quite a big issue considering that Signal is all about privacy...
- hutzlibu 5y ago
  Selective quoting? "As soon as we were able to pick up a scent, it was all we worked on, and we were able to get a fix out very quickly."
  - nathan_phoenix 5y ago
    Oh sorry, I interpreted it differently. Tho it still doesn't change anything, they prolonged investigating this issue for months and only put mayor work behind it when they "pick[ed] up a scent on it". Although they knew about it from day one (one Signal staff replied on the same day the issue was posted).
    - hutzlibu 5y ago
      Yeah but he explained, that they could not track down the bug, without having the luxory of default user tracking and metrics. So what can you do, when you have a non reproducible bug, but limited ressources(and other problems around)? You wait for the bug to show up again and then have more data to work with.
      - saurik 5y ago
        What you do is you tell everyone about the issue so they can take the appropriate steps to protect themselves, whatever that might be (don't send sensitive photos, don't use Signal to do it, only use Signal on iOS, etc.); they did not seem to do that.
        hutzlibu 5y ago
miken123 5y ago
I find this quite concerning and am really wondering if there are any privacy advantages to Signal if these things happen. Could you say something on: 1. Have any defend in depth measures been applied in the Android and other clients to make sure this issue does not happen again? E.g., an additional check when sending/encrypting a message to make sure it is absolutely meant for the person it is being send to? 2. Why did it take 8 months to fix this if some users could reproduce it consistently?
reinforcedpaper 5y ago
From the explanation of the bug on github it seems to me like this is a client-side database issue and nothing was actually leaked. Database ids were reused so random images that were previously received were displayed in newly received messages. Is this correct? If it is then it's probably worth mentioning.
- tgsovlerkhgsel 5y ago
  My understanding is that the database issue caused your client to send pictures A, B and C to person X, when you were trying to send picture C to person X (where A and B are pictures that were previously sent to someone else).
  - reinforcedpaper 5y ago
    The person reporting the issue specifically said that they couldn't find those pictures on their phone and don't remember ever sending them to anyone. The recipient also wouldn't be able to find those images anywhere else because they have chat trimming enabled. The result is that because a newly received message happened to share the id of an old deleted message, the new message is now displaying pictures from the old message. This does require the recipient to have received those pictures and also not remember them but I believe it is easier to forget a random picture you received than one you sent. Again, this is me speculating with very limited knowledge of client internals but it makes sense to me. I would like to see a developer confirm this.
eganist 5y ago
Question: do you guys have a software or product security team? I suggested the roles to workwithus @ Signal on 5/18/2018 and have never seen a public follow-up in the form of career postings. Asking because such a team may be best equipped to serve as both the support and internal accountability function for such while minimizing business conflicts when engineering is facing challenges integrating security into DevOps natively. At this point, it's probably warranted; the last time I asked was when Signal was seeing its spree of XSS defects in the desktop app. If Signal has one, a simple "yes" will suffice, but without a reply, I have to assume not.
- azornathogron 5y ago
  Given Signal's raison d'être, I would think nearly their entire team is the "security team". I'm not being entirely facetious either - security is the USP of the product, I really would expect security knowledge and a feeling of responsibility for the product's security to be pervasive throughout the whole team.
tylermenezes 5y ago
> we were able to get a fix out very quickly. Is 6 months really what Signal considers quick for a bug that leaks private data?
- hutzlibu 5y ago
  Selective quoting? "As soon as we were able to pick up a scent, it was all we worked on, and we were able to get a fix out very quickly."
  - nathan_phoenix 5y ago
    Does it change anything tho? They prolonged investigating this issue for months and only put mayor work behind it when they "pick[ed] up a scent on it". Although they knew about it from day one (one Signal staff replied on the same day the issue was posted).
  - tylermenezes 5y ago
    It's not at all selective. This should have been "all they worked on" from the moment they got several confirmations, not from the moment people beat them over the head with data. If they couldn't fix it they should have pulled the app. This is a company that aggressively markets itself to people needing privacy, and mistakes can ruin lives. And before you say it, they have tens of millions of dollars in funding.
    - hutzlibu 5y ago
      Well yes, maybe they should have put more people on it, from day one. But even though they have solid funding, doesn't mean they can throw it out the window. And non reproducible bugs can be hard, even when you throw money at them. But your quote was almost a textbook example of selective quoting, because you said, that they said they did a quick fix, when it really took over 6 months. But they did not say this - they said "once they pick up the scent" they delivered a quick fix. This is something very different.
      - 5y ago
rasengan 5y ago
You should have shut down the servers while this was happening - especially because you couldn't track it down. Shame on you for not caring about your users or their privacy when they were specifically using Signal for privacy. Edit: Anyone downvoting this also simply doesn't understand how serious this privacy leak really is. Shame on you too.
- jraph 5y ago
  > Edit: Anyone downvoting this also simply doesn't understand how serious this privacy leak really is. Shame on you too. Wouldn't showing a warning suggesting not to post sensitive images while the bug is being investigated be better than straight up shutting down the service while solving the privacy issue? Wouldn't closing the service have made people leave for other (probably worse) services, the one relying on privacy, privacy-minded people and "regular users" alike, and have worse consequences for privacy than this bug? (not currently a Signal user, and I did not downvote your comment)
- NavinF 5y ago
  It's pretty much impossible to distinguish rare bugs from user error if you don't have logging/telemetry.
deleted 5y ago
[deleted]
GekkePrutser 5y ago
One thing I wonder is: how could this happen at all? Considering the E2E Encryption in place, I would expect the incorrect recipient simply wouldn't be able to decode the image considering they never have exchanged keys with the sender?
- runarberg 5y ago
  Not a signal developer, but I would imagine it would be fairly simple that the same bug also encrypted the message with the eventual receiver’s key (as opposed to the intended receiver’s key). Resulting in a message which the eventual receiver could decrypt but not the intended receiver.
- alfiedotwtf 5y ago
  "How could this happen at all" > I remember before Jellybean Android, sending SMS would break up my message and send it to multiple people, and the Android alarm clock would drift by hours. So how could this happen? Because software is hard.
  - stathibus 5y ago
    Its not hard - its bad. Do better.
saurik 5y ago
Not mentioning in this comment when the issue was fixed--two weeks ago in the code and days ago in production--is extremely dishonest. I have been contacted by literally everyone I have sent this to saying something akin to "dude says it was fixed quickly but they just didn't close the issue" despite my message with the link saying (exactly) "<- issue opened on december 3rd and only fixed last week", making me have to again point out "closed today, but only fixed in production last week" (fixed in the code two weeks ago, but that doesn't matter much)... at which point they are forced to do a double take and suddenly care. I have worked on high-impact open source software--the iOS jailbreak ecosystem, writing some of the most core software for it, such as the mechanisms which support runtime code modification and which install the userland--with a much smaller team than Signal, and when you run into serious issues you need to disclose them, and you need to be super honest about the post-mortem. You shouldn't just kind of sweep the issue under the rug in the hope you can fix it before someone notices or it affects enough people to become a PR problem. (On what is maybe a side note of my thesis here for a moment, but for completeness on the related issue of why I am so dissatisfied with these PR-like responses: you say it is "extremely rare", but not so rare that tons of people aren't reporting the issue happening at least to people they know; this is being used here as an excuse for why it was hard to find and fix, but is then being taken by some as "oh it was also unimportant": issues have to be additionally weighted by their impact, and this bug was clearly critical.) The equivalent of this sort of thing I have run into is "there is like a one in a hundred thousand chance that you will experience catastrophic data loss from using my software", and I took those issues very seriously, as when you have tens of millions of users that's still a non-negligible number of people in the absolute: I considered every single person who would lose something like their camera roll on their phone to be a crushing defeat that I should internalize and take super personally, as I know the feeling of loss of important information and have enough empathy to assign it to my users. (Hell: one time I actually hired someone to spend a bunch of time going back and building a tool that would take videos recorded by Cycorder (my video recorder for the original iPhone) that had been damaged by a bug I found in one version of the app that had led to some videos being misrecorded and lost--in a way that I think was even more random than "merely" if the power ran out while recording?--and repair them, to send to the almost no people who had taken videos of their family they realized only after an event weren't playable. This is different, of course, as this was after the fact, but a demonstration of the empathy I feel developers should have for their users: if I can do it with the tiny resources I had... anyone can do it.) When you find reports of such an issue, you carefully track every single one of them down... but you also have a small time box, past which you need to disclose the issue to everyone: you put a large message on the download link of the product or update the homepage of the app to explain your status finding the issue and asking for help with leads, as it is important that people know that if this could affect them they can mitigate... maybe they don't send photos to anyone they couldn't afford sending to someone wrong, they use a different tool, or they switch to running Signal on iOS. This doesn't seem to have happened? Hell: if anything, this issue seems to have been sufficiently boring to you that you didn't even close it the second you fixed it or keep people abreast in the issue on GitHub of when they could expect the fix you committed to roll into production. This is both an unacceptable communication style and level of empathy for a product trying to be as important as Signal (though sadly not terribly surprising on either count... it is this same lack of empathy that springs up when Signal has database corruption issues or lacks export tooling or spends its time undermining the wrong opponents or throws in a cryptocurrency--which I should want to celebrate as I am in that space!!--built on DRM tooling and without any warning or thought as to what it means for the one open source secure messenger... sigh).
bradbeattie 5y ago
> I was tracking it on a separate issue Do you mind linking to that issue? Thanks!

TekMol 5y ago

Can you provide a link to the commit that fixes it? Shouldn't there have been an announcement to inform users what has been leaked and under which circumstances? How can user A send an image to user B that neither of them took? Isn't everything end-2-end encrypted? Then how can unencrypted data from user C end up on the device of user B?

agilob 5y ago
No PR with name that would suggest the fix in the client https://github.com/signalapp/Signal-Android/pulls?q=is%3Apr+author%3Agreyson-signal+is%3Aclosed https://github.com/signalapp/Signal-Android/pulls?q=is%3Apr+... and no PR from OP in the opensource part of the server https://github.com/signalapp/Signal-Server/pulls?q=is%3Apr+is%3Aclosed+author%3Agreyson-signal https://github.com/signalapp/Signal-Server/pulls?q=is%3Apr+i...
- deleted 5y ago
  [deleted]
- seg_lol 5y ago
  *edit, I think this issue was specific to the Android client, the desktop client has a totally different sqlite schema. The child comment to your comment is deleted, but I think autoincrement IDs shouldn't be used under an ambient authority context. It would make more sense to have IDs based on an LSF or Feistel sequence, perhaps split into a master ID and a conversation sequence. Autoincrement on this field makes it easy for off by one errors. Even just moving to guids and maintaining a proper parent child relationship would have prevented this. Or maybe there should be a database per conversation (set of all parties). https://github.com/signalapp/Signal-Android/commit/83086a5a2b8cae3ac40dea75d8c9533457a31858 https://github.com/signalapp/Signal-Android/commit/83086a5a2... https://github.com/signalapp/Signal-Android/commit/b9657208fea1c7bcfb90b7400037a7858ba56516 https://github.com/signalapp/Signal-Android/commit/b9657208f... Row IDs shouldn't have so much power.
  - WesolyKubeczek 5y ago
    The good thing about autoincrementing unsigned 64-bit integers is that 1) it's insanely fast, 2) SQLite is doing it automatically, 3) seriously, why don't they have it on all tables yet? SQLite guarantees no collisions within a table. Doing homegrown ID generation is how such bugs are being introduced in the first place. Your application level trigger got bypassed, oops. Your check was experimentally disabled and left like that for a year until people started noticing, oops. If you make an SQLite-backed application and it has a bug like this, I can safely bet $500 it's not going to be SQLite that has the bug. Just don't expose the numeric IDs in links, and generate your UUIDs in tables where you need to point to rows in an outside-accessible link. This is database 101, for crying out loud.
tomudding 5y ago
> Can you provide a link to the commit that fixes it? If I understand the issue [0] correctly, these two commits should be the fix: https://github.com/signalapp/Signal-Android/commit/e90fa05d608d49759e4cbac8cf233a797e0ee395 https://github.com/signalapp/Signal-Android/commit/e90fa05d6... https://github.com/signalapp/Signal-Android/commit/b9657208fea1c7bcfb90b7400037a7858ba56516 https://github.com/signalapp/Signal-Android/commit/b9657208f... The former updates how recipients (or really threads, I suppose) are merged (the issue occurred when trimming threads) and the latter changes the way how thread ids are generated (now automatically incremented). Together they should prevent unrelated recipients (threads) from being merged. [0]: https://github.com/signalapp/Signal-Android/issues/10247#issuecomment-886239978 https://github.com/signalapp/Signal-Android/issues/10247#iss...
- deleted 5y ago
  [deleted]
- winrid 5y ago
  I love the terrible commit name. "Updating recipient merging."
- hackinthebochs 5y ago
  Using incrementing ids as your source of ownership is just asking for trouble. This just means a programming error can have a high probability of ids lining up and leaking resources. Guids make this practically impossible.
  - rattray 5y ago
    Interesting, I hadn't thought of that advantage of uuids/guids before.
  - fulafel 5y ago
    I wonder if there's any widely implemented programming pattern that would catch this better, eg consisting of concatenated type code and id. Using GUIDs here would still hide the bug, not flag an error.
Popegaf 5y ago
From the explanation [0], this looks like the commit [1]. I do have to say though, the commit messages are very barren and barely any reference an issue directly. My guess is that they develop on private branches with an internal issue tracker and decide not to reference issues as that would be confusing. It would help a bunch if they referenced the public issues and made it clear which issues they're working on. They don't seem to use Github Projects [2]. All in all, the communication / transparency of the project is lacking - even though I'm glad they're providing something usable. Hopefully Matrix will be able to provide something as easily usable as well. [0]: https://github.com/signalapp/Signal-Android/issues/10247#issuecomment-886239978 https://github.com/signalapp/Signal-Android/issues/10247#iss... [1]: https://github.com/signalapp/Signal-Android/commit/83086a5a2b8cae3ac40dea75d8c9533457a31858 https://github.com/signalapp/Signal-Android/commit/83086a5a2... [2]: https://github.com/signalapp/Signal-Android/projects https://github.com/signalapp/Signal-Android/projects
konschubert 5y ago
If it’s a client bug that just switches out recipients, then the messages will just get encrypted for the wrong recipient.

lwhi 5y ago

I appreciate that this was a difficult and rare bug, but for an app that sells itself as 'secure', it feels like this isn't acceptable. How can users be assured that this type of issue won't occur again?

nerbert 5y ago
‘Many forms of secure messaging systems have been tried, and will be tried in this world of sin and woe. No one pretends that Signal is perfect or all-wise. Indeed it has been said that Signal is the worst messaging except for all those other forms that have been developed from time to time…’ Winston S Churchill, 11 November 1947
- geofft 5y ago
  Sure, but there's a small theoretical difference with democracy. You have to live under some system of government. You don't have to use a secure messenger. You can choose to have sensitive conversations in person or not have them at all. I agree that in practice, a lot of people are going to use their phones for relatively sensitive conversations, and in practice, Signal remains the best choice for doing so. But there are a few real threat models where the options aren't Signal vs. SMS / Google Chat / Discord / etc., the options are Signal vs. nothing. For instance, you could be a journalist deciding whether to ask clarifying questions to a government whistleblower via Signal or meet up with them in a park. You could be an activist/demonstrator under a repressive regime deciding whether to coordinate some action this weekend via Signal or hold off on it entirely and tactically preserve your freedom. And so forth. For those people, if (and to be clear this is a big "if," while this issue is one serious piece of evidence it is nonetheless inconclusive) Signal isn't trustworthy, it doesn't matter if Signal is the least-bad of the options. (Also, it's not like Signal is the only e2e messenger around. There's iMessage/FaceTime, for instance. Churchill's claim was that the abstract idea of democracy was good, not that any concrete implementation like the British government was good.)
  - helmholtz 5y ago
    You make some fair points. But even from the eyes of those people, what is the alternative? Is iMessage guaranteed to not have any hidden exploits out there? And on the flip side, what do they lose out on by only having those conversations in person? Well, I'd argue that their world becomes a lot smaller, and there sources are instantly at a higher risk.

andrewguenther 5y ago

> This bug was extraordinarily rare, and because we have no metrics/remote log collection, there was an initial period where we had to spend time adding logging and collecting user-submitted logs to try to track it down. Without telemetry, can you actually back up the claim that this issue was extremely rare?

hnarn 5y ago
Some details on how this assumption was made would be nice, but I think it's pretty obvious that any developer involved in a project can make a reasonable assumption of how rare a bug is depending on the technical details on what is required for the bug to happen. For example, if we say for the sake of argument that a hypothetical bug requires you to have more than ten contacts of the exact same name and these also need to share the same country and area code, one can make the assumption this use case is very rare without knowing the exact number of users that this applies to, just based on common sense regarding how the application is normally used. edit: The linked github issue says: > The TL;DR is that if someone had conversation trimming on, it could create a rare situation where a database ID was re-used in a way that could result in this behavior. It was very difficult to track down, with earlier phases involving getting additional logging into builds. Once we had some more information, it did in fact become our top priority, a fix was made, and we got it out as quickly and as safely as possible. The fix itself should make it so that database issues like the one that caused this bug can't happen again.
- NullPrefix 5y ago
  >for the sake of argument that a hypothetical bug requires you to have more than ten contacts of the exact same name and these also need to share the same country and area code This is only rare if you have a small social circle. My circle has multiple first name-collisions of at least 5 participants, but my circle is not very big and the area code is also quite small. Some countries do not use area codes for mobile phone numbers, which are used for Signal, meaning country code is the only area limiting factor.
  - 5y ago

spicybright 5y ago

Just wanted to say, thank you so much for your contributions. Signal is an amazing product, and things like these happen to the best of projects.

nathan_phoenix 5y ago

> we were able to get a fix out very quickly. I'm not sure if 8 months can be categorized as fast... The issue was posted on Dec 4, 2020, and the fix (5.17) was released on July 21. Also, sounds like quite a big issue considering that Signal is all about privacy...

hutzlibu 5y ago
Selective quoting? "As soon as we were able to pick up a scent, it was all we worked on, and we were able to get a fix out very quickly."
- nathan_phoenix 5y ago
  Oh sorry, I interpreted it differently. Tho it still doesn't change anything, they prolonged investigating this issue for months and only put mayor work behind it when they "pick[ed] up a scent on it". Although they knew about it from day one (one Signal staff replied on the same day the issue was posted).
  - hutzlibu 5y ago
    Yeah but he explained, that they could not track down the bug, without having the luxory of default user tracking and metrics. So what can you do, when you have a non reproducible bug, but limited ressources(and other problems around)? You wait for the bug to show up again and then have more data to work with.
    - saurik 5y ago
      What you do is you tell everyone about the issue so they can take the appropriate steps to protect themselves, whatever that might be (don't send sensitive photos, don't use Signal to do it, only use Signal on iOS, etc.); they did not seem to do that.
      - hutzlibu 5y ago

miken123 5y ago

I find this quite concerning and am really wondering if there are any privacy advantages to Signal if these things happen. Could you say something on: 1. Have any defend in depth measures been applied in the Android and other clients to make sure this issue does not happen again? E.g., an additional check when sending/encrypting a message to make sure it is absolutely meant for the person it is being send to? 2. Why did it take 8 months to fix this if some users could reproduce it consistently?

reinforcedpaper 5y ago

From the explanation of the bug on github it seems to me like this is a client-side database issue and nothing was actually leaked. Database ids were reused so random images that were previously received were displayed in newly received messages. Is this correct? If it is then it's probably worth mentioning.

tgsovlerkhgsel 5y ago
My understanding is that the database issue caused your client to send pictures A, B and C to person X, when you were trying to send picture C to person X (where A and B are pictures that were previously sent to someone else).
- reinforcedpaper 5y ago
  The person reporting the issue specifically said that they couldn't find those pictures on their phone and don't remember ever sending them to anyone. The recipient also wouldn't be able to find those images anywhere else because they have chat trimming enabled. The result is that because a newly received message happened to share the id of an old deleted message, the new message is now displaying pictures from the old message. This does require the recipient to have received those pictures and also not remember them but I believe it is easier to forget a random picture you received than one you sent. Again, this is me speculating with very limited knowledge of client internals but it makes sense to me. I would like to see a developer confirm this.

eganist 5y ago

Question: do you guys have a software or product security team? I suggested the roles to workwithus @ Signal on 5/18/2018 and have never seen a public follow-up in the form of career postings. Asking because such a team may be best equipped to serve as both the support and internal accountability function for such while minimizing business conflicts when engineering is facing challenges integrating security into DevOps natively. At this point, it's probably warranted; the last time I asked was when Signal was seeing its spree of XSS defects in the desktop app. If Signal has one, a simple "yes" will suffice, but without a reply, I have to assume not.

azornathogron 5y ago
Given Signal's raison d'être, I would think nearly their entire team is the "security team". I'm not being entirely facetious either - security is the USP of the product, I really would expect security knowledge and a feeling of responsibility for the product's security to be pervasive throughout the whole team.

tylermenezes 5y ago

> we were able to get a fix out very quickly. Is 6 months really what Signal considers quick for a bug that leaks private data?

hutzlibu 5y ago
Selective quoting? "As soon as we were able to pick up a scent, it was all we worked on, and we were able to get a fix out very quickly."
- nathan_phoenix 5y ago
  Does it change anything tho? They prolonged investigating this issue for months and only put mayor work behind it when they "pick[ed] up a scent on it". Although they knew about it from day one (one Signal staff replied on the same day the issue was posted).
- tylermenezes 5y ago
  It's not at all selective. This should have been "all they worked on" from the moment they got several confirmations, not from the moment people beat them over the head with data. If they couldn't fix it they should have pulled the app. This is a company that aggressively markets itself to people needing privacy, and mistakes can ruin lives. And before you say it, they have tens of millions of dollars in funding.
  - hutzlibu 5y ago
    Well yes, maybe they should have put more people on it, from day one. But even though they have solid funding, doesn't mean they can throw it out the window. And non reproducible bugs can be hard, even when you throw money at them. But your quote was almost a textbook example of selective quoting, because you said, that they said they did a quick fix, when it really took over 6 months. But they did not say this - they said "once they pick up the scent" they delivered a quick fix. This is something very different.
    - 5y ago

rasengan 5y ago

You should have shut down the servers while this was happening - especially because you couldn't track it down. Shame on you for not caring about your users or their privacy when they were specifically using Signal for privacy. Edit: Anyone downvoting this also simply doesn't understand how serious this privacy leak really is. Shame on you too.

jraph 5y ago
> Edit: Anyone downvoting this also simply doesn't understand how serious this privacy leak really is. Shame on you too. Wouldn't showing a warning suggesting not to post sensitive images while the bug is being investigated be better than straight up shutting down the service while solving the privacy issue? Wouldn't closing the service have made people leave for other (probably worse) services, the one relying on privacy, privacy-minded people and "regular users" alike, and have worse consequences for privacy than this bug? (not currently a Signal user, and I did not downvote your comment)
NavinF 5y ago
It's pretty much impossible to distinguish rare bugs from user error if you don't have logging/telemetry.

deleted 5y ago

[deleted]

GekkePrutser 5y ago

One thing I wonder is: how could this happen at all? Considering the E2E Encryption in place, I would expect the incorrect recipient simply wouldn't be able to decode the image considering they never have exchanged keys with the sender?

runarberg 5y ago
Not a signal developer, but I would imagine it would be fairly simple that the same bug also encrypted the message with the eventual receiver’s key (as opposed to the intended receiver’s key). Resulting in a message which the eventual receiver could decrypt but not the intended receiver.
alfiedotwtf 5y ago
"How could this happen at all" > I remember before Jellybean Android, sending SMS would break up my message and send it to multiple people, and the Android alarm clock would drift by hours. So how could this happen? Because software is hard.
- stathibus 5y ago
  Its not hard - its bad. Do better.

saurik 5y ago

Not mentioning in this comment when the issue was fixed--two weeks ago in the code and days ago in production--is extremely dishonest. I have been contacted by literally everyone I have sent this to saying something akin to "dude says it was fixed quickly but they just didn't close the issue" despite my message with the link saying (exactly) "<- issue opened on december 3rd and only fixed last week", making me have to again point out "closed today, but only fixed in production last week" (fixed in the code two weeks ago, but that doesn't matter much)... at which point they are forced to do a double take and suddenly care. I have worked on high-impact open source software--the iOS jailbreak ecosystem, writing some of the most core software for it, such as the mechanisms which support runtime code modification and which install the userland--with a much smaller team than Signal, and when you run into serious issues you need to disclose them, and you need to be super honest about the post-mortem. You shouldn't just kind of sweep the issue under the rug in the hope you can fix it before someone notices or it affects enough people to become a PR problem. (On what is maybe a side note of my thesis here for a moment, but for completeness on the related issue of why I am so dissatisfied with these PR-like responses: you say it is "extremely rare", but not so rare that tons of people aren't reporting the issue happening at least to people they know; this is being used here as an excuse for why it was hard to find and fix, but is then being taken by some as "oh it was also unimportant": issues have to be additionally weighted by their impact, and this bug was clearly critical.) The equivalent of this sort of thing I have run into is "there is like a one in a hundred thousand chance that you will experience catastrophic data loss from using my software", and I took those issues very seriously, as when you have tens of millions of users that's still a non-negligible number of people in the absolute: I considered every single person who would lose something like their camera roll on their phone to be a crushing defeat that I should internalize and take super personally, as I know the feeling of loss of important information and have enough empathy to assign it to my users. (Hell: one time I actually hired someone to spend a bunch of time going back and building a tool that would take videos recorded by Cycorder (my video recorder for the original iPhone) that had been damaged by a bug I found in one version of the app that had led to some videos being misrecorded and lost--in a way that I think was even more random than "merely" if the power ran out while recording?--and repair them, to send to the almost no people who had taken videos of their family they realized only after an event weren't playable. This is different, of course, as this was after the fact, but a demonstration of the empathy I feel developers should have for their users: if I can do it with the tiny resources I had... anyone can do it.) When you find reports of such an issue, you carefully track every single one of them down... but you also have a small time box, past which you need to disclose the issue to everyone: you put a large message on the download link of the product or update the homepage of the app to explain your status finding the issue and asking for help with leads, as it is important that people know that if this could affect them they can mitigate... maybe they don't send photos to anyone they couldn't afford sending to someone wrong, they use a different tool, or they switch to running Signal on iOS. This doesn't seem to have happened? Hell: if anything, this issue seems to have been sufficiently boring to you that you didn't even close it the second you fixed it or keep people abreast in the issue on GitHub of when they could expect the fix you committed to roll into production. This is both an unacceptable communication style and level of empathy for a product trying to be as important as Signal (though sadly not terribly surprising on either count... it is this same lack of empathy that springs up when Signal has database corruption issues or lacks export tooling or spends its time undermining the wrong opponents or throws in a cryptocurrency--which I should want to celebrate as I am in that space!!--built on DRM tooling and without any warning or thought as to what it means for the one open source secure messenger... sigh).

bradbeattie 5y ago

> I was tracking it on a separate issue Do you mind linking to that issue? Thanks!