Loading src/android/net/ip/IpClient.java +17 −3 Original line number Diff line number Diff line Loading @@ -64,7 +64,6 @@ import android.text.TextUtils; import android.util.LocalLog; import android.util.Log; import android.util.Pair; import android.util.Patterns; import android.util.SparseArray; import androidx.annotation.NonNull; Loading @@ -86,6 +85,8 @@ import com.android.server.NetworkStackService.NetworkStackServiceManager; import java.io.FileDescriptor; import java.io.PrintWriter; import java.net.InetAddress; import java.net.MalformedURLException; import java.net.URL; import java.nio.BufferUnderflowException; import java.nio.ByteBuffer; import java.util.ArrayList; Loading Loading @@ -1255,9 +1256,9 @@ public class IpClient extends StateMachine { } final String capportUrl = mDhcpResults.captivePortalApiUrl; // Uri.parse does no syntax check; do a simple regex check to eliminate garbage. // Uri.parse does no syntax check; do a simple check to eliminate garbage. // If the URL is still incorrect data fetching will fail later, which is fine. if (capportUrl != null && Patterns.WEB_URL.matcher(capportUrl).matches()) { if (isParseableUrl(capportUrl)) { NetworkInformationShimImpl.newInstance() .setCaptivePortalApiUrl(newLp, Uri.parse(capportUrl)); } Loading Loading @@ -1295,6 +1296,19 @@ public class IpClient extends StateMachine { return newLp; } private static boolean isParseableUrl(String url) { // Verify that a URL has a reasonable format that can be parsed as per the URL constructor. // This does not use Patterns.WEB_URL as that pattern excludes URLs without TLDs, such as on // localhost. if (url == null) return false; try { new URL(url); return true; } catch (MalformedURLException e) { return false; } } private static void addAllReachableDnsServers( LinkProperties lp, Iterable<InetAddress> dnses) { // TODO: Investigate deleting this reachability check. We should be Loading src/com/android/server/connectivity/NetworkMonitor.java +6 −1 Original line number Diff line number Diff line Loading @@ -2535,7 +2535,12 @@ public class NetworkMonitor extends StateMachine { final String apiContent; try { final URL url = new URL(mCaptivePortalApiUrl.toString()); if (!"https".equals(url.getProtocol())) { // Protocol must be HTTPS // (as per https://www.ietf.org/id/draft-ietf-capport-api-07.txt, #4). // Only allow HTTP on localhost, for testing. final boolean isLocalhostHttp = "localhost".equals(url.getHost()) && "http".equals(url.getProtocol()); if (!"https".equals(url.getProtocol()) && !isLocalhostHttp) { validationLog("Invalid captive portal API protocol: " + url.getProtocol()); return null; } Loading tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java +31 −1 Original line number Diff line number Diff line Loading @@ -1225,7 +1225,7 @@ public class NetworkMonitorTest { @Test public void testIsCaptivePortal_CapportApiInvalidContent() throws Exception { assumeTrue(CaptivePortalDataShimImpl.isSupported()); setStatus(mHttpsConnection, 204); setSslException(mHttpsConnection); setPortal302(mHttpConnection); setApiContent(mCapportApiConnection, "{SomeInvalidText"); runNetworkTest(makeCapportLPs(), CELL_METERED_CAPABILITIES, Loading @@ -1236,6 +1236,36 @@ public class NetworkMonitorTest { verify(mHttpConnection).getResponseCode(); } private void runCapportApiInvalidUrlTest(String url) throws Exception { assumeTrue(CaptivePortalDataShimImpl.isSupported()); setSslException(mHttpsConnection); setPortal302(mHttpConnection); final LinkProperties lp = new LinkProperties(TEST_LINK_PROPERTIES); lp.setCaptivePortalApiUrl(Uri.parse(url)); runNetworkTest(makeCapportLPs(), CELL_METERED_CAPABILITIES, VALIDATION_RESULT_PORTAL, 0 /* probesSucceeded */, TEST_LOGIN_URL); verify(mCallbacks, never()).notifyCaptivePortalDataChanged(any()); verify(mCapportApiConnection, never()).getInputStream(); verify(mHttpConnection).getResponseCode(); } @Test public void testIsCaptivePortal_HttpIsInvalidCapportApiScheme() throws Exception { runCapportApiInvalidUrlTest("http://capport.example.com"); } @Test public void testIsCaptivePortal_FileIsInvalidCapportApiScheme() throws Exception { runCapportApiInvalidUrlTest("file://localhost/myfile"); } @Test public void testIsCaptivePortal_InvalidUrlFormat() throws Exception { runCapportApiInvalidUrlTest("ThisIsNotAValidUrl"); } @Test @IgnoreUpTo(Build.VERSION_CODES.Q) public void testIsCaptivePortal_CapportApiNotSupported() throws Exception { // Test that on a R+ device, if NetworkStack was compiled without CaptivePortalData support Loading Loading
src/android/net/ip/IpClient.java +17 −3 Original line number Diff line number Diff line Loading @@ -64,7 +64,6 @@ import android.text.TextUtils; import android.util.LocalLog; import android.util.Log; import android.util.Pair; import android.util.Patterns; import android.util.SparseArray; import androidx.annotation.NonNull; Loading @@ -86,6 +85,8 @@ import com.android.server.NetworkStackService.NetworkStackServiceManager; import java.io.FileDescriptor; import java.io.PrintWriter; import java.net.InetAddress; import java.net.MalformedURLException; import java.net.URL; import java.nio.BufferUnderflowException; import java.nio.ByteBuffer; import java.util.ArrayList; Loading Loading @@ -1255,9 +1256,9 @@ public class IpClient extends StateMachine { } final String capportUrl = mDhcpResults.captivePortalApiUrl; // Uri.parse does no syntax check; do a simple regex check to eliminate garbage. // Uri.parse does no syntax check; do a simple check to eliminate garbage. // If the URL is still incorrect data fetching will fail later, which is fine. if (capportUrl != null && Patterns.WEB_URL.matcher(capportUrl).matches()) { if (isParseableUrl(capportUrl)) { NetworkInformationShimImpl.newInstance() .setCaptivePortalApiUrl(newLp, Uri.parse(capportUrl)); } Loading Loading @@ -1295,6 +1296,19 @@ public class IpClient extends StateMachine { return newLp; } private static boolean isParseableUrl(String url) { // Verify that a URL has a reasonable format that can be parsed as per the URL constructor. // This does not use Patterns.WEB_URL as that pattern excludes URLs without TLDs, such as on // localhost. if (url == null) return false; try { new URL(url); return true; } catch (MalformedURLException e) { return false; } } private static void addAllReachableDnsServers( LinkProperties lp, Iterable<InetAddress> dnses) { // TODO: Investigate deleting this reachability check. We should be Loading
src/com/android/server/connectivity/NetworkMonitor.java +6 −1 Original line number Diff line number Diff line Loading @@ -2535,7 +2535,12 @@ public class NetworkMonitor extends StateMachine { final String apiContent; try { final URL url = new URL(mCaptivePortalApiUrl.toString()); if (!"https".equals(url.getProtocol())) { // Protocol must be HTTPS // (as per https://www.ietf.org/id/draft-ietf-capport-api-07.txt, #4). // Only allow HTTP on localhost, for testing. final boolean isLocalhostHttp = "localhost".equals(url.getHost()) && "http".equals(url.getProtocol()); if (!"https".equals(url.getProtocol()) && !isLocalhostHttp) { validationLog("Invalid captive portal API protocol: " + url.getProtocol()); return null; } Loading
tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java +31 −1 Original line number Diff line number Diff line Loading @@ -1225,7 +1225,7 @@ public class NetworkMonitorTest { @Test public void testIsCaptivePortal_CapportApiInvalidContent() throws Exception { assumeTrue(CaptivePortalDataShimImpl.isSupported()); setStatus(mHttpsConnection, 204); setSslException(mHttpsConnection); setPortal302(mHttpConnection); setApiContent(mCapportApiConnection, "{SomeInvalidText"); runNetworkTest(makeCapportLPs(), CELL_METERED_CAPABILITIES, Loading @@ -1236,6 +1236,36 @@ public class NetworkMonitorTest { verify(mHttpConnection).getResponseCode(); } private void runCapportApiInvalidUrlTest(String url) throws Exception { assumeTrue(CaptivePortalDataShimImpl.isSupported()); setSslException(mHttpsConnection); setPortal302(mHttpConnection); final LinkProperties lp = new LinkProperties(TEST_LINK_PROPERTIES); lp.setCaptivePortalApiUrl(Uri.parse(url)); runNetworkTest(makeCapportLPs(), CELL_METERED_CAPABILITIES, VALIDATION_RESULT_PORTAL, 0 /* probesSucceeded */, TEST_LOGIN_URL); verify(mCallbacks, never()).notifyCaptivePortalDataChanged(any()); verify(mCapportApiConnection, never()).getInputStream(); verify(mHttpConnection).getResponseCode(); } @Test public void testIsCaptivePortal_HttpIsInvalidCapportApiScheme() throws Exception { runCapportApiInvalidUrlTest("http://capport.example.com"); } @Test public void testIsCaptivePortal_FileIsInvalidCapportApiScheme() throws Exception { runCapportApiInvalidUrlTest("file://localhost/myfile"); } @Test public void testIsCaptivePortal_InvalidUrlFormat() throws Exception { runCapportApiInvalidUrlTest("ThisIsNotAValidUrl"); } @Test @IgnoreUpTo(Build.VERSION_CODES.Q) public void testIsCaptivePortal_CapportApiNotSupported() throws Exception { // Test that on a R+ device, if NetworkStack was compiled without CaptivePortalData support Loading