From 7e762eb799abe0d4f172d04eb714b97e838a8b1f Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Wed, 30 Mar 2022 09:03:19 +0200 Subject: [PATCH 1/5] ensure downloaded file does not exceed Content-Length reported by HEAD --- .../http/HttpDownloadConnection.java | 23 ++++++++++++++++--- src/main/res/values/strings.xml | 1 + 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/http/HttpDownloadConnection.java b/src/main/java/eu/siacs/conversations/http/HttpDownloadConnection.java index 5623c0be7..31ba810a4 100644 --- a/src/main/java/eu/siacs/conversations/http/HttpDownloadConnection.java +++ b/src/main/java/eu/siacs/conversations/http/HttpDownloadConnection.java @@ -102,11 +102,15 @@ public class HttpDownloadConnection implements Transferable { if (this.message.getEncryption() == Message.ENCRYPTION_AXOLOTL && this.file.getKey() == null) { this.message.setEncryption(Message.ENCRYPTION_NONE); } - //TODO add auth tag size to knownFileSize final Long knownFileSize = message.getFileParams().size; Log.d(Config.LOGTAG,"knownFileSize: "+knownFileSize+", body="+message.getBody()); if (knownFileSize != null && interactive) { - this.file.setExpectedSize(knownFileSize); + if (message.getEncryption() == Message.ENCRYPTION_AXOLOTL + && this.file.getKey() != null) { + this.file.setExpectedSize(knownFileSize + 16); + } else { + this.file.setExpectedSize(knownFileSize); + } download(true); } else { checkFileSize(interactive); @@ -216,6 +220,8 @@ public class HttpDownloadConnection implements Transferable { mXmppConnectionService.showErrorToastInUi(R.string.download_failed_could_not_connect); } else if (e instanceof FileWriterException) { mXmppConnectionService.showErrorToastInUi(R.string.download_failed_could_not_write_file); + } else if (e instanceof InvalidFileException) { + mXmppConnectionService.showErrorToastInUi(R.string.download_failed_invalid_file); } else { mXmppConnectionService.showErrorToastInUi(R.string.download_failed_file_not_found); } @@ -428,9 +434,12 @@ public class HttpDownloadConnection implements Transferable { transmitted += count; try { outputStream.write(buffer, 0, count); - } catch (IOException e) { + } catch (final IOException e) { throw new FileWriterException(file); } + if (transmitted > expected) { + throw new InvalidFileException(String.format("File exceeds expected size of %d", expected)); + } updateProgress(Math.round(((double) transmitted / expected) * 100)); } outputStream.flush(); @@ -458,4 +467,12 @@ public class HttpDownloadConnection implements Transferable { throw new IOException(String.format(Locale.ENGLISH, "HTTP Status code was %d", code)); } } + + private static class InvalidFileException extends IOException { + + private InvalidFileException(final String message) { + super(message); + } + + } } diff --git a/src/main/res/values/strings.xml b/src/main/res/values/strings.xml index 8b5e67eb2..20c7cbef8 100644 --- a/src/main/res/values/strings.xml +++ b/src/main/res/values/strings.xml @@ -463,6 +463,7 @@ Download failed: File not found Download failed: Could not connect to host Download failed: Could not write file + Download failed: Invalid file Tor network unavailable Bind failure The server is not responsible for this domain From 09cf5feefa3a8a1ab21c84cb2208075ef216fc4a Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Wed, 30 Mar 2022 09:25:05 +0200 Subject: [PATCH 2/5] limit posh files to 10k --- .../services/MemorizingTrustManager.java | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/services/MemorizingTrustManager.java b/src/main/java/eu/siacs/conversations/services/MemorizingTrustManager.java index b51b8de41..520348943 100644 --- a/src/main/java/eu/siacs/conversations/services/MemorizingTrustManager.java +++ b/src/main/java/eu/siacs/conversations/services/MemorizingTrustManager.java @@ -43,6 +43,7 @@ import androidx.appcompat.app.AppCompatActivity; import com.google.common.base.Charsets; import com.google.common.base.Joiner; +import com.google.common.io.ByteStreams; import com.google.common.io.CharStreams; import org.json.JSONArray; @@ -77,6 +78,7 @@ import javax.net.ssl.TrustManager; import javax.net.ssl.TrustManagerFactory; import javax.net.ssl.X509TrustManager; +import eu.siacs.conversations.Config; import eu.siacs.conversations.R; import eu.siacs.conversations.crypto.XmppDomainVerifier; import eu.siacs.conversations.entities.MTMDecision; @@ -391,13 +393,13 @@ public class MemorizingTrustManager { final List fingerprints = getPoshFingerprints(domain); if (hash != null && fingerprints.size() > 0) { if (fingerprints.contains(hash)) { - Log.d("mtm", "trusted cert fingerprint of " + domain + " via posh"); + Log.d(Config.LOGTAG, "trusted cert fingerprint of " + domain + " via posh"); return; } else { - Log.d("mtm", "fingerprint " + hash + " not found in " + fingerprints); + Log.d(Config.LOGTAG, "fingerprint " + hash + " not found in " + fingerprints); } if (getPoshCacheFile(domain).delete()) { - Log.d("mtm", "deleted posh file for " + domain + " after not being able to verify"); + Log.d(Config.LOGTAG, "deleted posh file for " + domain + " after not being able to verify"); } } } @@ -410,7 +412,7 @@ public class MemorizingTrustManager { } } - private List getPoshFingerprints(String domain) { + private List getPoshFingerprints(final String domain) { final List cached = getPoshFingerprintsFromCache(domain); if (cached == null) { return getPoshFingerprintsFromServer(domain); @@ -424,13 +426,13 @@ public class MemorizingTrustManager { } private List getPoshFingerprintsFromServer(String domain, String url, int maxTtl, boolean followUrl) { - Log.d("mtm", "downloading json for " + domain + " from " + url); + Log.d(Config.LOGTAG, "downloading json for " + domain + " from " + url); final SharedPreferences preferences = PreferenceManager.getDefaultSharedPreferences(master); final boolean useTor = QuickConversationsService.isConversations() && preferences.getBoolean("use_tor", master.getResources().getBoolean(R.bool.use_tor)); try { final List results = new ArrayList<>(); final InputStream inputStream = HttpConnectionManager.open(url, useTor); - final String body = CharStreams.toString(new InputStreamReader(inputStream, Charsets.UTF_8)); + final String body = CharStreams.toString(new InputStreamReader(ByteStreams.limit(inputStream,10_000), Charsets.UTF_8)); final JSONObject jsonObject = new JSONObject(body); int expires = jsonObject.getInt("expires"); if (expires <= 0) { @@ -457,7 +459,7 @@ public class MemorizingTrustManager { writeFingerprintsToCache(domain, results, 1000L * expires + System.currentTimeMillis()); return results; } catch (final Exception e) { - Log.d("mtm", "error fetching posh " + e.getMessage()); + Log.d(Config.LOGTAG, "error fetching posh",e); return new ArrayList<>(); } } @@ -495,7 +497,7 @@ public class MemorizingTrustManager { file.delete(); return null; } else { - Log.d("mtm", "posh fingerprints expire in " + (expiresIn / 1000) + "s"); + Log.d(Config.LOGTAG, "posh fingerprints expire in " + (expiresIn / 1000) + "s"); } final List result = new ArrayList<>(); final JSONArray jsonArray = jsonObject.getJSONArray("fingerprints"); @@ -512,7 +514,6 @@ public class MemorizingTrustManager { } private X509Certificate[] getAcceptedIssuers() { - LOGGER.log(Level.FINE, "getAcceptedIssuers()"); return defaultTrustManager == null ? new X509Certificate[0] : defaultTrustManager.getAcceptedIssuers(); } From eadb1e127b81005b8d83a86197e6c71ce0115fcc Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Wed, 30 Mar 2022 09:59:42 +0200 Subject: [PATCH 3/5] disable knownFileSize on re-download for pgp encrypted files --- .../siacs/conversations/http/HttpDownloadConnection.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main/java/eu/siacs/conversations/http/HttpDownloadConnection.java b/src/main/java/eu/siacs/conversations/http/HttpDownloadConnection.java index 31ba810a4..032496842 100644 --- a/src/main/java/eu/siacs/conversations/http/HttpDownloadConnection.java +++ b/src/main/java/eu/siacs/conversations/http/HttpDownloadConnection.java @@ -102,7 +102,12 @@ public class HttpDownloadConnection implements Transferable { if (this.message.getEncryption() == Message.ENCRYPTION_AXOLOTL && this.file.getKey() == null) { this.message.setEncryption(Message.ENCRYPTION_NONE); } - final Long knownFileSize = message.getFileParams().size; + final Long knownFileSize; + if (message.getEncryption() == Message.ENCRYPTION_PGP || message.getEncryption() == Message.ENCRYPTION_DECRYPTED) { + knownFileSize = null; + } else { + knownFileSize = message.getFileParams().size; + } Log.d(Config.LOGTAG,"knownFileSize: "+knownFileSize+", body="+message.getBody()); if (knownFileSize != null && interactive) { if (message.getEncryption() == Message.ENCRYPTION_AXOLOTL From 95e3a6769d6cdc08ff86d70fb8cb561974346501 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Wed, 30 Mar 2022 18:45:18 +0200 Subject: [PATCH 4/5] retrieve uncompressed file size in HEAD request --- .../siacs/conversations/http/HttpDownloadConnection.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/http/HttpDownloadConnection.java b/src/main/java/eu/siacs/conversations/http/HttpDownloadConnection.java index 032496842..850c0683d 100644 --- a/src/main/java/eu/siacs/conversations/http/HttpDownloadConnection.java +++ b/src/main/java/eu/siacs/conversations/http/HttpDownloadConnection.java @@ -322,6 +322,7 @@ public class HttpDownloadConnection implements Transferable { ); final Request request = new Request.Builder() .url(URL.stripFragment(mUrl)) + .addHeader("Accept-Encoding", "identity") .head() .build(); mostRecentCall = client.newCall(request); @@ -347,11 +348,11 @@ public class HttpDownloadConnection implements Transferable { throw new IOException("Server reported negative file size"); } return size; - } catch (IOException e) { + } catch (final IOException e) { Log.d(Config.LOGTAG, "io exception during HEAD " + e.getMessage()); throw e; - } catch (NumberFormatException e) { - throw new IOException(); + } catch (final NumberFormatException e) { + throw new IOException(e); } } From 4af1fe39c79d4cf05e16776876ac386ceee288dd Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Thu, 31 Mar 2022 09:41:55 +0200 Subject: [PATCH 5/5] version bump to 2.10.5 + changelog --- CHANGELOG.md | 5 +++++ build.gradle | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b445c52ce..81c2d9e1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +### Version 2.10.5 + +* Security: Stop downloading files that exceed advertised file size +* Security: Limit POSH files to 10K + ### Version 2.10.4 * Fix interaction with Google Maps Share Location Plugin diff --git a/build.gradle b/build.gradle index b15db6067..766ac5e94 100644 --- a/build.gradle +++ b/build.gradle @@ -90,8 +90,8 @@ android { defaultConfig { minSdkVersion 21 targetSdkVersion 30 - versionCode 42028 - versionName "2.10.4" + versionCode 42031 + versionName "2.10.5" archivesBaseName += "-$versionName" applicationId "eu.siacs.conversations" resValue "string", "applicationId", applicationId