Skip to content

Commit

Permalink
DO NOT MERGE Ignore - Sanitized uri scheme by removing scheme delimiter
Browse files Browse the repository at this point in the history
Initially considered removing unsupported characters as per IANA guidelines, but this could break applications that use custom schemes with asterisks. Instead, opted to remove only the "://" to minimize disruption

Bug: 261721900
Test: atest FrameworksCoreTests:android.net.UriTest

No-Typo-Check: The unit test is specifically written to test few cases, string "http://https://" is not a typo

NOTE FOR REVIEWERS - original patch and result patch are not identical.
PLEASE REVIEW CAREFULLY.
Diffs between the patches:
     @AsbSecurityTest(cveBugId = 261721900)
> +    @smallTest
> +    public void testSchemeSanitization() {
> +        Uri uri = new Uri.Builder()
> +                .scheme("http://https://evil.com:/te:st/")
> +                .authority("google.com").path("one/way").build();
> +        assertEquals("httphttpsevil.com:/te:st/", uri.getScheme());
> +        assertEquals("httphttpsevil.com:/te:st/://google.com/one/way", uri.toString());
> +    }
> +

Original patch:
 diff --git a/core/java/android/net/Uri.java b/core/java/android/net/Uri.java
old mode 100644
new mode 100644
--- a/core/java/android/net/Uri.java
+++ b/core/java/android/net/Uri.java
@@ -1388,7 +1388,11 @@
          * @param scheme name or {@code null} if this is a relative Uri
          */
         public Builder scheme(String scheme) {
-            this.scheme = scheme;
+            if (scheme != null) {
+                this.scheme = scheme.replace("://", "");
+            } else {
+                this.scheme = null;
+            }
             return this;
         }

diff --git a/core/tests/coretests/src/android/net/UriTest.java b/core/tests/coretests/src/android/net/UriTest.java
old mode 100644
new mode 100644
--- a/core/tests/coretests/src/android/net/UriTest.java
+++ b/core/tests/coretests/src/android/net/UriTest.java
@@ -87,6 +87,16 @@
         assertNull(u.getAuthority());
         assertNull(u.getHost());
     }
+
+    @AsbSecurityTest(cveBugId = 261721900)
+    @smallTest
+    public void testSc
[[[Original patch trimmed due to size. Decoded string size: 1426. Decoded string SHA1: 55d69e9f854938457b2d98b18776898b16c2dd54.]]]

Result patch:
 diff --git a/core/java/android/net/Uri.java b/core/java/android/net/Uri.java
index 3da696a..f0262e9 100644
--- a/core/java/android/net/Uri.java
+++ b/core/java/android/net/Uri.java
@@ -1388,7 +1388,11 @@
          * @param scheme name or {@code null} if this is a relative Uri
          */
         public Builder scheme(String scheme) {
-            this.scheme = scheme;
+            if (scheme != null) {
+                this.scheme = scheme.replace("://", "");
+            } else {
+                this.scheme = null;
+            }
             return this;
         }

diff --git a/core/tests/coretests/src/android/net/UriTest.java b/core/tests/coretests/src/android/net/UriTest.java
index 89632a4..8c130ee 100644
--- a/core/tests/coretests/src/android/net/UriTest.java
+++ b/core/tests/coretests/src/android/net/UriTest.java
@@ -88,6 +88,16 @@
         assertNull(u.getHost());
     }

+    @AsbSecurityTest(cveBugId = 261721900)
+    @smallTest
+    public void testSchemeSanitization() {
+        Uri uri = new
[[[Result patch trimmed due to size. Decoded string size: 1417. Decoded string SHA1: f9ce831a369872ae9bfd9f50f01dd394682e0f3f.]]]
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:841ce92aa1b350c83148ef6fb57bfff617364e1a)
Merged-In: Icab100bd4ae9b1c8245e6f891ad22101bda5eea5
Change-Id: Icab100bd4ae9b1c8245e6f891ad22101bda5eea5
  • Loading branch information
Kiran Ramachandra authored and aoleary committed Sep 17, 2024
1 parent 21ea000 commit 566bbf0
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 1 deletion.
6 changes: 5 additions & 1 deletion core/java/android/net/Uri.java
Original file line number Diff line number Diff line change
Expand Up @@ -1388,7 +1388,11 @@ public Builder() {}
* @param scheme name or {@code null} if this is a relative Uri
*/
public Builder scheme(String scheme) {
this.scheme = scheme;
if (scheme != null) {
this.scheme = scheme.replace("://", "");
} else {
this.scheme = null;
}
return this;
}

Expand Down
11 changes: 11 additions & 0 deletions core/tests/coretests/src/android/net/UriTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import android.content.ContentUris;
import android.os.Parcel;
import android.platform.test.annotations.AsbSecurityTest;

import androidx.test.filters.SmallTest;

Expand Down Expand Up @@ -88,6 +89,16 @@ public void testBuildUponOpaqueStringUri() {
assertNull(u.getHost());
}

@AsbSecurityTest(cveBugId = 261721900)
@SmallTest
public void testSchemeSanitization() {
Uri uri = new Uri.Builder()
.scheme("http://https://evil.com:/te:st/")
.authority("google.com").path("one/way").build();
assertEquals("httphttpsevil.com:/te:st/", uri.getScheme());
assertEquals("httphttpsevil.com:/te:st/://google.com/one/way", uri.toString());
}

@SmallTest
public void testStringUri() {
assertEquals("bob lee",
Expand Down

0 comments on commit 566bbf0

Please sign in to comment.