From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Lucas Lin <lucaslin@google.com>
Date: Fri, 3 Mar 2023 08:13:50 +0000
Subject: [PATCH] Sanitize VPN label to prevent HTML injection

This commit will try to sanitize the content of VpnDialog. This
commit creates a function which will try to sanitize the VPN
label, if the sanitized VPN label is different from the original
one, which means the VPN label might contain HTML tag or the VPN
label violates the words restriction(may contain some wording
which will mislead the user). For this kind of case, show the
package name instead of the VPN label to prevent misleading the
user.

The malicious VPN app might be able to add a large number of line
breaks with HTML in order to hide the system-displayed text from
the user in the connection request dialog. Thus, sanitizing the
content of the dialog is needed.

Bug: 204554636
Test: N/A
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:2178216b98bf9865edee198f45192f0b883624ab)
Merged-In: I8eb890fd2e5797d8d6ab5b12f9c628bc9616081d
Change-Id: I8eb890fd2e5797d8d6ab5b12f9c628bc9616081d
---
 packages/VpnDialogs/res/values/strings.xml    | 28 ++++++++++
 .../com/android/vpndialogs/ConfirmDialog.java | 53 +++++++++++++++++--
 2 files changed, 76 insertions(+), 5 deletions(-)

diff --git a/packages/VpnDialogs/res/values/strings.xml b/packages/VpnDialogs/res/values/strings.xml
index 443a9bc33b90..b4166f0bedfd 100644
--- a/packages/VpnDialogs/res/values/strings.xml
+++ b/packages/VpnDialogs/res/values/strings.xml
@@ -89,4 +89,32 @@
          without any consequences. [CHAR LIMIT=20] -->
     <string name="dismiss">Dismiss</string>
 
+    <!-- Malicious VPN apps may provide very long labels or cunning HTML to trick the system dialogs
+         into displaying what they want. The system will attempt to sanitize the label, and if the
+         label is deemed dangerous, then this string is used instead. The first argument is the
+         first 30 characters of the label, and the second argument is the package name of the app.
+         Example : Normally a VPN app may be called "My VPN app" in which case the dialog will read
+         "My VPN app wants to set up a VPN connection...". If the label is very long, then, this
+         will be used to show "VerylongVPNlabel… (com.my.vpn.app) wants to set up a VPN
+         connection...". For this case, the code will refer to sanitized_vpn_label_with_ellipsis.
+    -->
+    <string name="sanitized_vpn_label_with_ellipsis">
+        <xliff:g id="sanitized_vpn_label_with_ellipsis" example="My VPN app">%1$s</xliff:g>… (
+        <xliff:g id="sanitized_vpn_label_with_ellipsis" example="com.my.vpn.app">%2$s</xliff:g>)
+    </string>
+
+    <!-- Malicious VPN apps may provide very long labels or cunning HTML to trick the system dialogs
+         into displaying what they want. The system will attempt to sanitize the label, and if the
+         label is deemed dangerous, then this string is used instead. The first argument is the
+         label, and the second argument is the package name of the app.
+         Example : Normally a VPN app may be called "My VPN app" in which case the dialog will read
+         "My VPN app wants to set up a VPN connection...". If the VPN label contains HTML tag but
+         the length is not very long, the dialog will show "VpnLabelWith&lt;br&gt;HtmlTag
+         (com.my.vpn.app) wants to set up a VPN connection...". For this case, the code will refer
+         to sanitized_vpn_label.
+    -->
+    <string name="sanitized_vpn_label">
+        <xliff:g id="sanitized_vpn_label" example="My VPN app">%1$s</xliff:g> (
+        <xliff:g id="sanitized_vpn_label" example="com.my.vpn.app">%2$s</xliff:g>)
+    </string>
 </resources>
diff --git a/packages/VpnDialogs/src/com/android/vpndialogs/ConfirmDialog.java b/packages/VpnDialogs/src/com/android/vpndialogs/ConfirmDialog.java
index 09339743db5c..43d18df3a10d 100644
--- a/packages/VpnDialogs/src/com/android/vpndialogs/ConfirmDialog.java
+++ b/packages/VpnDialogs/src/com/android/vpndialogs/ConfirmDialog.java
@@ -42,10 +42,52 @@ public class ConfirmDialog extends AlertActivity
         implements DialogInterface.OnClickListener, ImageGetter {
     private static final String TAG = "VpnConfirm";
 
+    // Usually the label represents the app name, 150 code points might be enough to display the app
+    // name, and 150 code points won't cover the warning message from VpnDialog.
+    static final int MAX_VPN_LABEL_LENGTH = 150;
+
     private String mPackage;
 
     private IConnectivityManager mService;
 
+    private View mView;
+
+    /**
+     * This function will use the string resource to combine the VPN label and the package name.
+     *
+     * If the VPN label violates the length restriction, the first 30 code points of VPN label and
+     * the package name will be returned. Or return the VPN label and the package name directly if
+     * the VPN label doesn't violate the length restriction.
+     *
+     * The result will be something like,
+     * - ThisIsAVeryLongVpnAppNameWhich... (com.vpn.app)
+     *   if the VPN label violates the length restriction.
+     * or
+     * - VpnLabelWith&lt;br&gt;HtmlTag (com.vpn.app)
+     *   if the VPN label doesn't violate the length restriction.
+     *
+     */
+    private String getSimplifiedLabel(String vpnLabel, String packageName) {
+        if (vpnLabel.codePointCount(0, vpnLabel.length()) > 30) {
+            return getString(R.string.sanitized_vpn_label_with_ellipsis,
+                vpnLabel.substring(0, vpnLabel.offsetByCodePoints(0, 30)),
+                packageName);
+        }
+
+        return getString(R.string.sanitized_vpn_label, vpnLabel, packageName);
+    }
+
+    protected String getSanitizedVpnLabel(String vpnLabel, String packageName) {
+        final String sanitizedVpnLabel = Html.escapeHtml(vpnLabel);
+        final boolean exceedMaxVpnLabelLength = sanitizedVpnLabel.codePointCount(0,
+            sanitizedVpnLabel.length()) > MAX_VPN_LABEL_LENGTH;
+        if (exceedMaxVpnLabelLength || !vpnLabel.equals(sanitizedVpnLabel)) {
+            return getSimplifiedLabel(sanitizedVpnLabel, packageName);
+        }
+
+        return sanitizedVpnLabel;
+    }
+
     @Override
     protected void onCreate(Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
@@ -68,15 +110,16 @@ public class ConfirmDialog extends AlertActivity
             finish();
             return;
         }
-        View view = View.inflate(this, R.layout.confirm, null);
-        ((TextView) view.findViewById(R.id.warning)).setText(
-                Html.fromHtml(getString(R.string.warning, getVpnLabel()),
-                        this, null /* tagHandler */));
+        mView = View.inflate(this, R.layout.confirm, null);
+        ((TextView) mView.findViewById(R.id.warning)).setText(
+                Html.fromHtml(getString(R.string.warning, getSanitizedVpnLabel(
+                    getVpnLabel().toString(), mPackage)),
+                    this /* imageGetter */, null /* tagHandler */));
         mAlertParams.mTitle = getText(R.string.prompt);
         mAlertParams.mPositiveButtonText = getText(android.R.string.ok);
         mAlertParams.mPositiveButtonListener = this;
         mAlertParams.mNegativeButtonText = getText(android.R.string.cancel);
-        mAlertParams.mView = view;
+        mAlertParams.mView = mView;
         setupAlert();
 
         getWindow().setCloseOnTouchOutside(false);