From b0889b2e44ea28f1ef15d64d37da44b5ff94399d Mon Sep 17 00:00:00 2001
From: damith_samarakoon <damith_samarakoon@persistent.com>
Date: Tue, 25 Feb 2025 16:52:09 +0530
Subject: [PATCH] Code fixes for CVE-2023-24442

---
 .../githubprcoveragestatus/Configuration.java | 63 ++++++++++++-------
 .../ConfigurationTest.java                    | 39 ++++++++++++
 2 files changed, 79 insertions(+), 23 deletions(-)
 create mode 100644 src/test/java/com/github/terma/jenkins/githubprcoveragestatus/ConfigurationTest.java

diff --git a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/Configuration.java b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/Configuration.java
index e8ead18..0a7f382 100644
--- a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/Configuration.java
+++ b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/Configuration.java
@@ -2,19 +2,20 @@
 
     Copyright 2015-2016 Artem Stasiuk
 
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
+    Licensed under the Apache License, Version 2.0 (the "License");
+    you may not use this file except in compliance with the License.
+    You may obtain a copy of the License at
 
-    http://www.apache.org/licenses/LICENSE-2.0
+        http://www.apache.org/licenses/LICENSE-2.0
 
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
+    Unless required by applicable law or agreed to in writing, software
+    distributed under the License is distributed on an "AS IS" BASIS,
+    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+    See the License for the specific language governing permissions and
+    limitations under the License.
 
 */
+
 package com.github.terma.jenkins.githubprcoveragestatus;
 
 import hudson.Extension;
@@ -27,12 +28,12 @@
 import org.apache.commons.lang.math.NumberUtils;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.StaplerRequest;
+import org.kohsuke.stapler.FormException;
 
 import edu.umd.cs.findbugs.annotations.NonNull;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 
-@SuppressWarnings("WeakerAccess")
 public class Configuration extends AbstractDescribableImpl<Configuration> {
 
     @Extension
@@ -97,14 +98,14 @@ public static final class ConfigurationDescriptor extends Descriptor<Configurati
 
         private boolean disableSimpleCov;
         private String gitHubApiUrl;
-        private String personalAccessToken;
+        private Secret personalAccessToken;
         private String jenkinsUrl;
         private boolean privateJenkinsPublicGitHub;
         private boolean useSonarForMasterCoverage;
         private String sonarUrl;
-        private String sonarToken;
+        private Secret sonarToken;
         private String sonarLogin;
-        private String sonarPassword;
+        private Secret sonarPassword;
 
         private int yellowThreshold = DEFAULT_YELLOW_THRESHOLD;
         private int greenThreshold = DEFAULT_GREEN_THRESHOLD;
@@ -135,7 +136,7 @@ public String getGitHubApiUrl() {
 
         @Override
         public String getPersonalAccessToken() {
-            return personalAccessToken;
+            return personalAccessToken == null ? null : personalAccessToken.getPlainText();
         }
 
         @Override
@@ -170,7 +171,7 @@ public String getSonarUrl() {
 
         @Override
         public String getSonarToken() {
-            return sonarToken;
+            return sonarToken == null ? null : sonarToken.getPlainText();
         }
 
         @Override
@@ -183,14 +184,13 @@ public String getSonarLogin() {
         }
 
         public String getSonarPassword() {
-            return sonarPassword;
+            return sonarPassword == null ? null : sonarPassword.getPlainText();
         }
 
         @Override
         public boolean configure(StaplerRequest req, JSONObject formData) throws FormException {
             gitHubApiUrl = StringUtils.trimToNull(formData.getString("gitHubApiUrl"));
-            personalAccessToken = Secret.toString(Secret.fromString(
-                    StringUtils.trimToNull(formData.getString("personalAccessToken"))));
+            personalAccessToken = Secret.fromString(StringUtils.trimToNull(formData.getString("personalAccessToken")));
             yellowThreshold = NumberUtils.toInt(formData.getString("yellowThreshold"), DEFAULT_YELLOW_THRESHOLD);
             greenThreshold = NumberUtils.toInt(formData.getString("greenThreshold"), DEFAULT_GREEN_THRESHOLD);
             jenkinsUrl = StringUtils.trimToNull(formData.getString("jenkinsUrl"));
@@ -198,15 +198,32 @@ public boolean configure(StaplerRequest req, JSONObject formData) throws FormExc
             useSonarForMasterCoverage = BooleanUtils.toBoolean(formData.getString("useSonarForMasterCoverage"));
             disableSimpleCov = BooleanUtils.toBoolean(formData.getString("disableSimpleCov"));
             sonarUrl = StringUtils.trimToNull(formData.getString("sonarUrl"));
-            sonarToken = Secret.toString(Secret.fromString(
-                    StringUtils.trimToNull(formData.getString("sonarToken"))));
+            sonarToken = Secret.fromString(StringUtils.trimToNull(formData.getString("sonarToken")));
             sonarLogin = StringUtils.trimToNull(formData.getString("sonarLogin"));
-            sonarPassword = Secret.toString(Secret.fromString(
-                    StringUtils.trimToNull(formData.getString("sonarPassword"))));
+            sonarPassword = Secret.fromString(StringUtils.trimToNull(formData.getString("sonarPassword")));
             save();
             return super.configure(req, formData);
         }
 
-    }
+        @Override
+        public void load() {
+            super.load();
+            // Re-wrap secret fields using Secret.fromString to ensure encrypted data storage
+            if (personalAccessToken != null) {
+                personalAccessToken = Secret.fromString(personalAccessToken.getPlainText());
+            }
+            if (sonarToken != null) {
+                sonarToken = Secret.fromString(sonarToken.getPlainText());
+            }
+            if (sonarPassword != null) {
+                sonarPassword = Secret.fromString(sonarPassword.getPlainText());
+            }
+        }
 
+        @Override
+        public void save() {
+            // Additional processing for secret objects can be applied here if needed
+            super.save();
+        }
+    }
 }
diff --git a/src/test/java/com/github/terma/jenkins/githubprcoveragestatus/ConfigurationTest.java b/src/test/java/com/github/terma/jenkins/githubprcoveragestatus/ConfigurationTest.java
new file mode 100644
index 0000000..d5f1356
--- /dev/null
+++ b/src/test/java/com/github/terma/jenkins/githubprcoveragestatus/ConfigurationTest.java
@@ -0,0 +1,39 @@
+package com.github.terma.jenkins.githubprcoveragestatus;
+
+import hudson.util.Secret;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+public class ConfigurationTest {
+
+    @Test
+    public void testEncryptionAndDecryption() {
+        String sensitiveData = "mySecretPassword";
+        Secret secret = Secret.fromString(sensitiveData);
+        String decrypted = secret.getPlainText();
+        assertEquals("The decrypted data should match the original sensitive data", sensitiveData, decrypted);
+    }
+
+    @Test
+    public void testConfigurationSensitiveField() {
+        String sensitive = "anotherSecret";
+        // Assuming a Configuration class with a default constructor and methods to set and get a sensitive field
+        Configuration config = new Configuration();
+        config.setSensitiveField(Secret.fromString(sensitive));
+        Secret retrievedSecret = config.getSensitiveField();
+        assertNotNull("Sensitive field should not be null", retrievedSecret);
+        assertEquals("The sensitive field should be stored and retrieved correctly in plaintext", sensitive, retrievedSecret.getPlainText());
+    }
+
+    @Test
+    public void testMultipleEncryptionDecryption() {
+        String[] sensitiveValues = {"password123", "tokenValue", "secretKey!"};
+        for (String value : sensitiveValues) {
+            Secret secret = Secret.fromString(value);
+            String decrypted = secret.getPlainText();
+            assertEquals("Decrypted text should match the original for value: " + value, value, decrypted);
+        }
+    }
+}