Skip to content

Commit

Permalink
fix: command injection vulnerability
Browse files Browse the repository at this point in the history
due to missing quoting, command injection was possible via
pipeline configuration.

This is now fixed using a quoting and escaping utility.
  • Loading branch information
holgpar committed Oct 24, 2024
1 parent 4990b2d commit cac6794
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 1 deletion.
17 changes: 17 additions & 0 deletions test/groovy/TestsPublishResultsTest.groovy
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import com.sap.piper.BashUtils
import org.junit.After
import org.junit.Before
import org.junit.Ignore
Expand All @@ -9,8 +10,12 @@ import org.junit.rules.ExpectedException
import util.BasePiperTest
import util.JenkinsReadYamlRule
import util.JenkinsStepRule

import static org.hamcrest.Matchers.not
import static org.junit.Assert.assertEquals
import static org.junit.Assert.assertThat
import static org.junit.Assert.assertTrue
import static org.hamcrest.Matchers.containsString

import com.sap.piper.Utils

Expand Down Expand Up @@ -230,4 +235,16 @@ class TestsPublishResultsTest extends BasePiperTest {

stepRule.step.testsPublishResults(script: nullScript, failOnError: true)
}

@Test
void testPublishUnitTestsWithUpdateResultsDoesNotAllowCommandExecution() throws Exception {
def injectString = "' -exec touch {} ; rm -rf / # –"
helper.registerAllowedMethod('sh', [String], { String cmd ->
assertThat(cmd, containsString(BashUtils.quoteAndEscape(injectString)))
})

stepRule.step.testsPublishResults(script: nullScript, junit: [pattern: injectString, archive: true, active: true, updateResults: true])


}
}
3 changes: 2 additions & 1 deletion vars/testsPublishResults.groovy
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import static com.sap.piper.Prerequisites.checkScript
import static com.sap.piper.BashUtils.quoteAndEscape as q

import com.sap.piper.GenerateDocumentation
import com.sap.piper.ConfigurationHelper
Expand Down Expand Up @@ -193,7 +194,7 @@ void touchFiles(pattern){
echo "[${STEP_NAME}] update test results"
def patternArray = pattern.split(',')
for(def i = 0; i < patternArray.length; i++){
sh "find . -wholename '${patternArray[i].trim()}' -exec touch {} \\;"
sh "find . -wholename ${q(patternArray[i].trim())} -exec touch {} \\;"
}
}

Expand Down

0 comments on commit cac6794

Please sign in to comment.