Fix int overflow in shift
Thankfully this is not actually a security vulnerability, because
the sign bit just gets set, and the conversion to long performs
sign extension. Still, it would break when adding one with 32.
To avoid repeats of this same mistake, convert the creation of
masks to using a function in a way that makes it impossible.
Test: new test for this in NetworkCapabilitiesUtilsTest
Change-Id: Iadbbca6b09b02d98e45a43984f7749e1a795ce02
diff --git a/staticlibs/framework/com/android/net/module/util/NetworkCapabilitiesUtils.java b/staticlibs/framework/com/android/net/module/util/NetworkCapabilitiesUtils.java
index d6222a7..71a0c96 100644
--- a/staticlibs/framework/com/android/net/module/util/NetworkCapabilitiesUtils.java
+++ b/staticlibs/framework/com/android/net/module/util/NetworkCapabilitiesUtils.java
@@ -27,6 +27,7 @@
import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET;
import static android.net.NetworkCapabilities.NET_CAPABILITY_MCX;
import static android.net.NetworkCapabilities.NET_CAPABILITY_MMS;
+import static android.net.NetworkCapabilities.NET_CAPABILITY_MMTEL;
import static android.net.NetworkCapabilities.NET_CAPABILITY_OEM_PAID;
import static android.net.NetworkCapabilities.NET_CAPABILITY_OEM_PRIVATE;
import static android.net.NetworkCapabilities.NET_CAPABILITY_RCS;
@@ -84,40 +85,41 @@
* and {@code FORCE_RESTRICTED_CAPABILITIES}.
*/
@VisibleForTesting
- static final long RESTRICTED_CAPABILITIES =
- (1 << NET_CAPABILITY_BIP)
- | (1 << NET_CAPABILITY_CBS)
- | (1 << NET_CAPABILITY_DUN)
- | (1 << NET_CAPABILITY_EIMS)
- | (1 << NET_CAPABILITY_ENTERPRISE)
- | (1 << NET_CAPABILITY_FOTA)
- | (1 << NET_CAPABILITY_IA)
- | (1 << NET_CAPABILITY_IMS)
- | (1 << NET_CAPABILITY_MCX)
- | (1 << NET_CAPABILITY_RCS)
- | (1 << NET_CAPABILITY_VEHICLE_INTERNAL)
- | (1 << NET_CAPABILITY_VSIM)
- | (1 << NET_CAPABILITY_XCAP);
+ static final long RESTRICTED_CAPABILITIES = packBitList(
+ NET_CAPABILITY_BIP,
+ NET_CAPABILITY_CBS,
+ NET_CAPABILITY_DUN,
+ NET_CAPABILITY_EIMS,
+ NET_CAPABILITY_ENTERPRISE,
+ NET_CAPABILITY_FOTA,
+ NET_CAPABILITY_IA,
+ NET_CAPABILITY_IMS,
+ NET_CAPABILITY_MCX,
+ NET_CAPABILITY_RCS,
+ NET_CAPABILITY_VEHICLE_INTERNAL,
+ NET_CAPABILITY_VSIM,
+ NET_CAPABILITY_XCAP,
+ NET_CAPABILITY_MMTEL);
/**
* Capabilities that force network to be restricted.
* See {@code NetworkCapabilities#maybeMarkCapabilitiesRestricted}.
*/
- private static final long FORCE_RESTRICTED_CAPABILITIES =
- (1 << NET_CAPABILITY_ENTERPRISE)
- | (1 << NET_CAPABILITY_OEM_PAID)
- | (1 << NET_CAPABILITY_OEM_PRIVATE);
+ private static final long FORCE_RESTRICTED_CAPABILITIES = packBitList(
+ NET_CAPABILITY_ENTERPRISE,
+ NET_CAPABILITY_OEM_PAID,
+ NET_CAPABILITY_OEM_PRIVATE);
/**
* Capabilities that suggest that a network is unrestricted.
* See {@code NetworkCapabilities#maybeMarkCapabilitiesRestricted}.
*/
@VisibleForTesting
- static final long UNRESTRICTED_CAPABILITIES =
- (1 << NET_CAPABILITY_INTERNET)
- | (1 << NET_CAPABILITY_MMS)
- | (1 << NET_CAPABILITY_SUPL)
- | (1 << NET_CAPABILITY_WIFI_P2P);
+ static final long UNRESTRICTED_CAPABILITIES = packBitList(
+ NET_CAPABILITY_INTERNET,
+ NET_CAPABILITY_MMS,
+ NET_CAPABILITY_SUPL,
+ NET_CAPABILITY_WIFI_P2P);
/**
* Get a transport that can be used to classify a network when displaying its info to users.
@@ -196,7 +198,26 @@
}
/**
+ * Packs a list of ints in the same way as packBits()
+ *
+ * Each passed int is the rank of a bit that should be set in the returned long.
+ * Example : passing (1,3) will return in 0b00001010 and passing (5,6,0) will return 0b01100001
+ *
+ * @param bits bits to pack
+ * @return a long with the specified bits set.
+ */
+ public static long packBitList(int... bits) {
+ return packBits(bits);
+ }
+
+ /**
* Packs array of bits into a long value.
+ *
+ * Each passed int is the rank of a bit that should be set in the returned long.
+ * Example : passing [1,3] will return in 0b00001010 and passing [5,6,0] will return 0b01100001
+ *
+ * @param bits bits to pack
+ * @return a long with the specified bits set.
*/
public static long packBits(int[] bits) {
long packed = 0;
diff --git a/staticlibs/tests/unit/src/com/android/net/module/util/NetworkCapabilitiesUtilsTest.kt b/staticlibs/tests/unit/src/com/android/net/module/util/NetworkCapabilitiesUtilsTest.kt
index f78c74e..256ea1e 100644
--- a/staticlibs/tests/unit/src/com/android/net/module/util/NetworkCapabilitiesUtilsTest.kt
+++ b/staticlibs/tests/unit/src/com/android/net/module/util/NetworkCapabilitiesUtilsTest.kt
@@ -18,6 +18,7 @@
import android.annotation.TargetApi
import android.net.NetworkCapabilities
+import android.net.NetworkCapabilities.NET_CAPABILITY_BIP
import android.net.NetworkCapabilities.NET_CAPABILITY_CBS
import android.net.NetworkCapabilities.NET_CAPABILITY_EIMS
import android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET
@@ -33,6 +34,7 @@
import android.os.Build
import androidx.test.filters.SmallTest
import androidx.test.runner.AndroidJUnit4
+import com.android.modules.utils.build.SdkLevel
import com.android.net.module.util.NetworkCapabilitiesUtils.RESTRICTED_CAPABILITIES
import com.android.net.module.util.NetworkCapabilitiesUtils.UNRESTRICTED_CAPABILITIES
import com.android.net.module.util.NetworkCapabilitiesUtils.getDisplayTransport
@@ -110,6 +112,12 @@
// as restricted when there is no any unrestricted capability.
nc.removeCapability(NET_CAPABILITY_INTERNET)
assertTrue(NetworkCapabilitiesUtils.inferRestrictedCapability(nc))
+ if (!SdkLevel.isAtLeastS()) return
+ // BIP deserves its specific test because it's the first capability over 30, meaning the
+ // shift will overflow
+ nc.removeCapability(NET_CAPABILITY_CBS)
+ nc.addCapability(NET_CAPABILITY_BIP)
+ assertTrue(NetworkCapabilitiesUtils.inferRestrictedCapability(nc))
}
@Test
@@ -122,9 +130,17 @@
assertEquals((1 shl NET_CAPABILITY_CBS).toLong() and RESTRICTED_CAPABILITIES,
(1 shl NET_CAPABILITY_CBS).toLong())
+ // verify BIP is also restricted
+ // BIP is not available in R and before, but the BIP constant is inlined so
+ // this test can still run on R.
+ assertEquals((1L shl NET_CAPABILITY_BIP) and RESTRICTED_CAPABILITIES,
+ (1L shl NET_CAPABILITY_BIP))
+
// verify default is not restricted
assertEquals((1 shl NET_CAPABILITY_INTERNET).toLong() and RESTRICTED_CAPABILITIES, 0)
+ assertTrue(RESTRICTED_CAPABILITIES > 0)
+
// just to see
assertEquals(RESTRICTED_CAPABILITIES and UNRESTRICTED_CAPABILITIES, 0)
}