commit a66e7d350d54bc1f769f42444f914955781bcc37 Author: Karsten Loesing karsten.loesing@gmx.net Date: Wed Sep 28 17:06:51 2016 +0200

Validate base64 input more carefully before parsing. --- .../bridgedescs/SanitizedBridgesWriter.java | 10 ++++++++++ .../bridgedescs/SanitizedBridgesWriterTest.java | 23 ++++++++++++++++++++++ 2 files changed, 33 insertions(+)

diff --git a/src/main/java/org/torproject/collector/bridgedescs/SanitizedBridgesWriter.java b/src/main/java/org/torproject/collector/bridgedescs/SanitizedBridgesWriter.java index abec743..d93cd90 100644 --- a/src/main/java/org/torproject/collector/bridgedescs/SanitizedBridgesWriter.java +++ b/src/main/java/org/torproject/collector/bridgedescs/SanitizedBridgesWriter.java @@ -479,6 +479,11 @@ public class SanitizedBridgesWriter extends CollecTorMain { + "status. Skipping descriptor."); return; } + if (!Base64.isBase64(parts[2])) { + logger.warn("Illegal base64 character in r line '" + parts[2] + + "'. Skipping descriptor."); + return; + } fingerprintBytes = Base64.decodeBase64(parts[2] + "=="); descPublicationTime = parts[4] + " " + parts[5]; String address = parts[6]; @@ -776,6 +781,11 @@ public class SanitizedBridgesWriter extends CollecTorMain { scrubbed.append("extra-info-digest " + DigestUtils.shaHex( Hex.decodeHex(parts[1].toCharArray())).toUpperCase()); if (parts.length > 2) { + if (!Base64.isBase64(parts[2])) { + logger.warn("Illegal base64 character in extra-info-digest line '" + + line + "'. Skipping descriptor."); + return; + } scrubbed.append(" " + Base64.encodeBase64String( DigestUtils.sha256(Base64.decodeBase64(parts[2]))) .replaceAll("=", "")); diff --git a/src/test/java/org/torproject/collector/bridgedescs/SanitizedBridgesWriterTest.java b/src/test/java/org/torproject/collector/bridgedescs/SanitizedBridgesWriterTest.java index 9f3857f..e248b10 100644 --- a/src/test/java/org/torproject/collector/bridgedescs/SanitizedBridgesWriterTest.java +++ b/src/test/java/org/torproject/collector/bridgedescs/SanitizedBridgesWriterTest.java @@ -301,6 +301,18 @@ public class SanitizedBridgesWriterTest { }

@Test + public void testServerDescriptorExtraInfoDigestInvalidBase64() + throws Exception { + this.defaultServerDescriptorBuilder.replaceLineStartingWith( + "extra-info-digest ", Arrays.asList("extra-info-digest " + + "6D03E80568DEFA102968D144CB35FFA6E3355B8A " + + "#*?$%§@nxukmmcT1+UnDg4qh0yKbjVUYKhGL8VksoJA")); + this.runTest(); + assertTrue("Invalid base64 in server descriptor accepted.", + this.parsedServerDescriptors.isEmpty()); + } + + @Test public void testServerDescriptorExtraInfoDigestSha1Only() throws Exception { this.defaultServerDescriptorBuilder.replaceLineStartingWith( @@ -497,6 +509,17 @@ public class SanitizedBridgesWriterTest { }

@Test + public void testNetworkStatusRlineInvalidBase64() throws Exception { + this.defaultNetworkStatusBuilder.replaceLineStartingWith("r ", + Arrays.asList("r MeekGoogle R#SnE*e4+lFag:xr_XxSL+J;ZVs " + + "g+M7'w+lG$mv6NW9&RmvzLO(R0Y 2016-06-30 21:43:52 " + + "198.50.200.131 8008 0")); + this.runTest(); + assertTrue("Should not have accepted invalid base64.", + this.parsedNetworkStatuses.isEmpty()); + } + + @Test public void testNetworkStatusAlinePortMissing() throws Exception { this.configuration.setProperty(Key.ReplaceIpAddressesWithHashes.name(), "true");