From dca26b0a5d78e4c24eb8598de082fa789c4a0077 Mon Sep 17 00:00:00 2001
From: Astralidea <astralidea@163.com>
Date: Fri, 18 Oct 2024 10:53:02 -0700
Subject: [PATCH] Fix known XSS vulnerabilities

Signed-off-by: Astralidea <astralidea@163.com>
---
 fe/fe-core/pom.xml                                   |  6 ++++++
 .../java/com/starrocks/http/action/LogAction.java    | 12 +++++++-----
 .../starrocks/http/action/QueryProfileAction.java    |  4 ++--
 .../java/com/starrocks/http/action/SystemAction.java |  5 ++++-
 fe/pom.xml                                           |  7 +++++++
 5 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/fe/fe-core/pom.xml b/fe/fe-core/pom.xml
index a709f9b1d418a..62b855072cec5 100644
--- a/fe/fe-core/pom.xml
+++ b/fe/fe-core/pom.xml
@@ -1048,6 +1048,12 @@ under the License.
             <artifactId>odps-sdk-table-api</artifactId>
         </dependency>
 
+        <!-- https://mvnrepository.com/artifact/org.owasp.encoder/encoder -->
+        <dependency>
+            <groupId>org.owasp.encoder</groupId>
+            <artifactId>encoder</artifactId>
+        </dependency>
+
         <dependency>
             <groupId>com.carrotsearch</groupId>
             <artifactId>junit-benchmarks</artifactId>
diff --git a/fe/fe-core/src/main/java/com/starrocks/http/action/LogAction.java b/fe/fe-core/src/main/java/com/starrocks/http/action/LogAction.java
index e4873beb2e584..ae39f72d6ecac 100644
--- a/fe/fe-core/src/main/java/com/starrocks/http/action/LogAction.java
+++ b/fe/fe-core/src/main/java/com/starrocks/http/action/LogAction.java
@@ -30,6 +30,7 @@
 import org.apache.commons.lang.StringUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.owasp.encoder.Encode;
 
 import java.io.FileNotFoundException;
 import java.io.IOException;
@@ -55,9 +56,9 @@ public static void registerAction(ActionController controller) throws IllegalArg
     public void executeGet(BaseRequest request, BaseResponse response) {
         getPageHeader(request, response.getContent());
 
-        // get parameters
-        addVerboseName = request.getSingleParameter("add_verbose");
-        delVerboseName = request.getSingleParameter("del_verbose");
+        // HTML encode the add_verbose and del_verbose to prevent XSS
+        addVerboseName = Encode.forHtml(request.getSingleParameter("add_verbose"));
+        delVerboseName = Encode.forHtml(request.getSingleParameter("del_verbose"));
         LOG.info("add verbose name: {}, del verbose name: {}", addVerboseName, delVerboseName);
 
         appendLogConf(response.getContent());
@@ -141,9 +142,10 @@ private void appendLogInfo(StringBuilder buffer) {
             raf.seek(startPos);
             buffer.append("<p>Showing last " + webContentLength + " bytes of log</p>");
             buffer.append("<pre>");
-            String fileBuffer = null;
+            String fileBuffer;
             while ((fileBuffer = raf.readLine()) != null) {
-                buffer.append(fileBuffer).append("\n");
+                // HTML encode to prevent XSS
+                buffer.append(Encode.forHtml(fileBuffer)).append("\n");
             }
             buffer.append("</pre>");
         } catch (FileNotFoundException e) {
diff --git a/fe/fe-core/src/main/java/com/starrocks/http/action/QueryProfileAction.java b/fe/fe-core/src/main/java/com/starrocks/http/action/QueryProfileAction.java
index 37068a6d26795..5539efbc9f1df 100644
--- a/fe/fe-core/src/main/java/com/starrocks/http/action/QueryProfileAction.java
+++ b/fe/fe-core/src/main/java/com/starrocks/http/action/QueryProfileAction.java
@@ -42,9 +42,9 @@
 import com.starrocks.http.IllegalArgException;
 import io.netty.handler.codec.http.HttpMethod;
 import io.netty.handler.codec.http.HttpResponseStatus;
-import org.apache.commons.text.StringEscapeUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.owasp.encoder.Encode;
 
 import java.io.BufferedReader;
 import java.io.IOException;
@@ -75,7 +75,7 @@ public void executeGet(BaseRequest request, BaseResponse response) {
         }
 
         // HTML encode the queryId to prevent XSS
-        String encodedQueryId = StringEscapeUtils.escapeHtml4(queryId);
+        String encodedQueryId = Encode.forHtml(queryId);
         String queryProfileStr = ProfileManager.getInstance().getProfile(queryId);
         if (queryProfileStr != null) {
             appendCopyButton(response.getContent());
diff --git a/fe/fe-core/src/main/java/com/starrocks/http/action/SystemAction.java b/fe/fe-core/src/main/java/com/starrocks/http/action/SystemAction.java
index 5ca09a73b2268..8b45343c63397 100644
--- a/fe/fe-core/src/main/java/com/starrocks/http/action/SystemAction.java
+++ b/fe/fe-core/src/main/java/com/starrocks/http/action/SystemAction.java
@@ -54,6 +54,7 @@
 import org.apache.commons.validator.routines.UrlValidator;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.owasp.encoder.Encode;
 
 import java.util.List;
 import java.util.stream.Collectors;
@@ -77,7 +78,9 @@ public void executeGet(BaseRequest request, BaseResponse response) {
         if (Strings.isNullOrEmpty(currentPath)) {
             currentPath = "/";
         }
-        appendSystemInfo(response.getContent(), currentPath, currentPath);
+        // HTML encode the path to prevent XSS
+        String encodePath = Encode.forHtml(currentPath);
+        appendSystemInfo(response.getContent(), encodePath, encodePath);
 
         getPageFooter(response.getContent());
         writeResponse(request, response);
diff --git a/fe/pom.xml b/fe/pom.xml
index 229a0d2d145fb..740404f5b6f7c 100644
--- a/fe/pom.xml
+++ b/fe/pom.xml
@@ -871,6 +871,13 @@ under the License.
                 <version>${odps.version}</version>
             </dependency>
 
+            <!-- https://mvnrepository.com/artifact/org.owasp.encoder/encoder -->
+            <dependency>
+                <groupId>org.owasp.encoder</groupId>
+                <artifactId>encoder</artifactId>
+                <version>1.3.1</version>
+            </dependency>
+
         </dependencies>
     </dependencyManagement>