Skip to content

Commit

Permalink
[SPARK-38992][CORE] Avoid using bash -c in ShellBasedGroupsMappingPro…
Browse files Browse the repository at this point in the history
…vider

### What changes were proposed in this pull request?

This PR proposes to avoid using `bash -c` in `ShellBasedGroupsMappingProvider`. This could allow users a command injection.

### Why are the changes needed?

For a security purpose.

### Does this PR introduce _any_ user-facing change?

Virtually no.

### How was this patch tested?

Manually tested.

Closes #36315 from HyukjinKwon/SPARK-38992.

Authored-by: Hyukjin Kwon <gurwls223@apache.org>
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
(cherry picked from commit c83618e)
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
  • Loading branch information
HyukjinKwon committed Apr 22, 2022
1 parent 0fdb675 commit 9cc2ae7
Showing 1 changed file with 3 additions and 2 deletions.
Expand Up @@ -30,6 +30,8 @@ import org.apache.spark.util.Utils
private[spark] class ShellBasedGroupsMappingProvider extends GroupMappingServiceProvider
with Logging {

private lazy val idPath = Utils.executeAndGetOutput("which" :: "id" :: Nil).stripLineEnd

override def getGroups(username: String): Set[String] = {
val userGroups = getUnixGroups(username)
logDebug("User: " + username + " Groups: " + userGroups.mkString(","))
Expand All @@ -38,8 +40,7 @@ private[spark] class ShellBasedGroupsMappingProvider extends GroupMappingService

// shells out a "bash -c id -Gn username" to get user groups
private def getUnixGroups(username: String): Set[String] = {
val cmdSeq = Seq("bash", "-c", "id -Gn " + username)
// we need to get rid of the trailing "\n" from the result of command execution
Utils.executeAndGetOutput(cmdSeq).stripLineEnd.split(" ").toSet
Utils.executeAndGetOutput(idPath :: "-Gn" :: username :: Nil).stripLineEnd.split(" ").toSet
}
}

0 comments on commit 9cc2ae7

Please sign in to comment.