SECURITY-3611

uhafner · Kevin-CB · commit 1dfe888b0249 · 2025-12-03T15:01:41.000+01:00
diff --git a/plugin/src/main/java/io/jenkins/plugins/coverage/metrics/steps/CoverageBuildAction.java b/plugin/src/main/java/io/jenkins/plugins/coverage/metrics/steps/CoverageBuildAction.java
@@ -40,6 +40,7 @@
 import io.jenkins.plugins.util.JenkinsFacade;
 import io.jenkins.plugins.util.JobAction;
 import io.jenkins.plugins.util.QualityGateResult;
+import io.jenkins.plugins.util.ValidationUtilities;
 
 import static hudson.model.Run.*;
 
@@ -59,6 +60,7 @@ public final class CoverageBuildAction extends BuildAction<Node> implements Stap
 private static final String NO_REFERENCE_BUILD = "-";
 private static final List<Difference> NO_VALUES = List.of();
 private static final int MAX_METRICS_COUNT_IN_SUMMARY = 5;
+ private static final ValidationUtilities VALIDATION_UTILITIES = new ValidationUtilities();
 
 private final String id;
 private final String name;
@@ -210,6 +212,8 @@ public CoverageBuildAction(final Run<?, ?> owner, final String id, final String
 final boolean canSerialize){
 super(owner, result, false);
 
+ VALIDATION_UTILITIES.ensureValidId(id);
+
 this.id = id;
 this.name = name;
 this.icon = icon;
@@ -241,6 +245,8 @@ private <T> ArrayList<T> copy(final List<? extends T> list){
 protected Object readResolve(){
 super.readResolve();
 
+ VALIDATION_UTILITIES.ensureValidId(id);
+
 if (difference == null){
 difference = new TreeMap<>();
 }
diff --git a/plugin/src/test/java/io/jenkins/plugins/coverage/metrics/steps/CoverageBuildActionTest.java b/plugin/src/test/java/io/jenkins/plugins/coverage/metrics/steps/CoverageBuildActionTest.java
@@ -4,6 +4,7 @@
 import org.apache.commons.lang3.math.Fraction;
 import org.junit.jupiter.api.Test;
 import org.junitpioneer.jupiter.DefaultLocale;
+import org.junitpioneer.jupiter.Issue;
 
 import edu.hm.hafner.coverage.Coverage.CoverageBuilder;
 import edu.hm.hafner.coverage.Difference;
@@ -21,6 +22,7 @@
 
 import io.jenkins.plugins.coverage.metrics.model.Baseline;
 import io.jenkins.plugins.util.QualityGateResult;
+import io.jenkins.plugins.util.QualityGateStatus;
 
 import static org.assertj.core.api.Assertions.*;
 import static org.mockito.Mockito.*;
@@ -32,6 +34,16 @@
 */
 @DefaultLocale("en")
 class CoverageBuildActionTest{
+ @Test
+ @Issue("SECURITY-3611")
+ void shouldValidateInAction(){
+ String evilId = "javascript:alert(1)";
+ assertThatIllegalArgumentException().isThrownBy(() ->
+ new CoverageBuildAction(mock(FreeStyleBuild.class), evilId, "name", "icon",
+ new ModuleNode("root"), new QualityGateResult(QualityGateStatus.ERROR), new FilteredLog()))
+ .withMessageContaining("An ID must match the regexp pattern");
+ }
+
 @Test
 void shouldNotLoadResultIfCoverageValuesArePersistedInAction(){
 var module = new ModuleNode("module");

-Original file line number
+Diff line change
 importio.jenkins.plugins.util.JenkinsFacade;
 importio.jenkins.plugins.util.JobAction;
 importio.jenkins.plugins.util.QualityGateResult;
 +importio.jenkins.plugins.util.ValidationUtilities;
 importstatichudson.model.Run.*;
 privatestaticfinalStringNO_REFERENCE_BUILD = "-";
 privatestaticfinalList<Difference> NO_VALUES = List.of();
 privatestaticfinalintMAX_METRICS_COUNT_IN_SUMMARY = 5;
 +privatestaticfinalValidationUtilitiesVALIDATION_UTILITIES = newValidationUtilities();
 privatefinalStringid;
 privatefinalStringname;
 finalbooleancanSerialize){
 super(owner, result, false);
 +VALIDATION_UTILITIES.ensureValidId(id);
++
 this.id = id;
 this.name = name;
 this.icon = icon;
 protectedObjectreadResolve(){
 super.readResolve();
 +VALIDATION_UTILITIES.ensureValidId(id);
++
 if (difference == null){
 difference = newTreeMap<>();
+ }
-Original file line number
+Diff line change
 importorg.apache.commons.lang3.math.Fraction;
 importorg.junit.jupiter.api.Test;
 importorg.junitpioneer.jupiter.DefaultLocale;
 +importorg.junitpioneer.jupiter.Issue;
 importedu.hm.hafner.coverage.Coverage.CoverageBuilder;
 importedu.hm.hafner.coverage.Difference;
 importio.jenkins.plugins.coverage.metrics.model.Baseline;
 importio.jenkins.plugins.util.QualityGateResult;
 +importio.jenkins.plugins.util.QualityGateStatus;
 importstaticorg.assertj.core.api.Assertions.*;
 importstaticorg.mockito.Mockito.*;
  */
 @DefaultLocale("en")
 classCoverageBuildActionTest{
 +@Test
 +@Issue("SECURITY-3611")
 +voidshouldValidateInAction(){
 +StringevilId = "javascript:alert(1)";
 +assertThatIllegalArgumentException().isThrownBy(() ->
 +newCoverageBuildAction(mock(FreeStyleBuild.class), evilId, "name", "icon",
 +newModuleNode("root"), newQualityGateResult(QualityGateStatus.ERROR), newFilteredLog()))
 + .withMessageContaining("An ID must match the regexp pattern");
 + }
++
 @Test
 voidshouldNotLoadResultIfCoverageValuesArePersistedInAction(){
 varmodule = newModuleNode("module");