Inline images with invalid HTTPS certs cause error that prevents use of stream

[WesleyAC]

Loading a inline image with a invalid HTTPS cert causes the following:

While this doesn't appear to have ever happened in the wild in Sentry (???), and changes in Camo in 4.0 will make it impossible for people to construct messages like this in the future, we should still fix it for the sake of being able to view existing messages.

#integrations > create svg will repro this.

(marking a-Android, since I don't think this would affect iOS, and P1 since it's a DoS vector on pre-6b7a3fb74 servers.)

[chrisbobbe]

I know of two other issues that seem like probably the same thing: #4337, #4691. (I'm getting caught up on things from last week's call, so I don't think I'll have time to deduplicate just now; there's some discussion to read through.)

[timabbott]

Just updating this thread that we apparently have a similar bug in the desktop app (zulip/zulip-desktop#1119), and that we should still consider this important since (1) Camo doesn't run in docker-zulip, so a lot of 4.0 servers are still affected and (2) inability to read old content before the Camo changes is important in its own right.

[WesleyAC]

This was fixed upstream react-native-webview/react-native-webview#1466, which we pulled in in 945848d. Closing.

[chrisbobbe]

This was fixed upstream react-native-webview/react-native-webview#1466, which we pulled in in 945848d. Closing.

Hmm, are you sure this has been fixed? Your screenshot at the top of the issue was from 2021-05-06, and the release containing 945848d, v27.155, went out on 2020-09-23.

[WesleyAC]

Ah, makes sense. I'm pretty sure it's been fixed, since editing the HTML-creation code to add <img src="https://expired.badssl.com/test.png"> to the output HTML rendered just fine (as a broken image). I wasn't able to reproduce this, so I went looking for an upstream fix and found react-native-webview/react-native-webview#1466, which did fix it. I got 945848d from git blame on package.json, but I think we actually got it in a yarn.lock update, likely 78a7763.

Thanks for catching that!

[chrisbobbe]

I think we actually got it in a yarn.lock update, likely 78a7763

Great, thanks for that investigation! Indeed, the react-native-webview PR says "This PR is included in version 10.9.3", and we went from 10.8.3 to 10.10.2 in that commit (link to changed lines in yarn.lock).