[tor-commits] [collector/master] Validate base64 input more carefully before parsing.

[karsten at torproject.org]

commit a66e7d350d54bc1f769f42444f914955781bcc37
Author: Karsten Loesing <karsten.loesing at 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");