Skip to content

Commit

Permalink
[Fix SECURITY-445] better protect temporary files
Browse files Browse the repository at this point in the history 
Temporary files were previously written to the system temporary directory
with default permissions.  A malicious actor could have captured sensitive
information by reading files from the temporary directory.  The temporary
files typically are only on the file system for the duration of a single
command line git invocation, but cloning a large git repo could require
an extended time with those sensitive files in the temporary directory.

This change sets permissions on the temporary files to be readable only by
the file owner. If a workspace is available, a temporary directory adjacent
to the workspace is used instead of the system temporary directory.
  • Loading branch information
@MarkEWaite
MarkEWaite committed Apr 24, 2017
1 parent 716e3ff commit 75ea3fe
Showing 1 changed file with 42 additions and 9 deletions.
51 changes: 42 additions & 9 deletions src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@
import java.net.URISyntaxException;
import java.nio.charset.Charset;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.attribute.FileAttribute;
import java.nio.file.attribute.PosixFilePermission;
import java.nio.file.attribute.PosixFilePermissions;
import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -1411,6 +1416,34 @@ public void addNote(String note, String namespace ) throws GitException, Interru
createNote(note,namespace,"add");
}

private File createTempFileInSystemDir(String prefix, String suffix) throws IOException {
if (isWindows()) {
return Files.createTempFile(prefix, suffix).toFile();
}
Set<PosixFilePermission> ownerOnly = PosixFilePermissions.fromString("rw-------");
FileAttribute fileAttribute = PosixFilePermissions.asFileAttribute(ownerOnly);
return Files.createTempFile(prefix, suffix, fileAttribute).toFile();
}

private File createTempFile(String prefix, String suffix) throws IOException {
if (workspace == null) {
return createTempFileInSystemDir(prefix, suffix);
}
File workspaceTmp = new File(workspace.getAbsolutePath() + "@tmp");
if (!workspaceTmp.isDirectory() && !workspaceTmp.mkdirs()) {
if (!workspaceTmp.isDirectory()) {
return createTempFileInSystemDir(prefix, suffix);
}
}
Path tmpPath = Paths.get(workspaceTmp.getAbsolutePath());
if (isWindows()) {
return Files.createTempFile(tmpPath, prefix, suffix).toFile();
}
Set<PosixFilePermission> ownerOnly = PosixFilePermissions.fromString("rw-------");
FileAttribute fileAttribute = PosixFilePermissions.asFileAttribute(ownerOnly);
return Files.createTempFile(tmpPath, prefix, suffix, fileAttribute).toFile();
}

private void deleteTempFile(File tempFile) {
if (tempFile != null && !tempFile.delete() && tempFile.exists()) {
listener.getLogger().println("[WARNING] temp file " + tempFile + " not deleted");
Expand All @@ -1420,7 +1453,7 @@ private void deleteTempFile(File tempFile) {
private void createNote(String note, String namespace, String command ) throws GitException, InterruptedException {
File msg = null;
try {
msg = File.createTempFile("git-note", "txt", workspace);
msg = createTempFile("git-note", ".txt");
FileUtils.writeStringToFile(msg,note);
launchCommand("notes", "--ref=" + namespace, command, "-F", msg.getAbsolutePath());
} catch (IOException | GitException e) {
Expand Down Expand Up @@ -1561,7 +1594,7 @@ private String launchCommandWithCredentials(ArgumentListBuilder args, File workD
}

private File createSshKeyFile(SSHUserPrivateKey sshUser) throws IOException, InterruptedException {
File key = File.createTempFile("ssh", "key");
File key = createTempFile("ssh", ".key");
try (PrintWriter w = new PrintWriter(key, Charset.defaultCharset().toString())) {
List<String> privateKeys = sshUser.getPrivateKeys();
for (String s : privateKeys) {
Expand Down Expand Up @@ -1597,7 +1630,7 @@ private String quoteUnixCredentials(String str) {
}

private File createWindowsSshAskpass(SSHUserPrivateKey sshUser) throws IOException {
File ssh = File.createTempFile("pass", ".bat");
File ssh = createTempFile("pass", ".bat");
try (PrintWriter w = new PrintWriter(ssh, Charset.defaultCharset().toString())) {
// avoid echoing command as part of the password
w.println("@echo off");
Expand All @@ -1610,7 +1643,7 @@ private File createWindowsSshAskpass(SSHUserPrivateKey sshUser) throws IOExcepti
}

private File createUnixSshAskpass(SSHUserPrivateKey sshUser) throws IOException {
File ssh = File.createTempFile("pass", ".sh");
File ssh = createTempFile("pass", ".sh");
try (PrintWriter w = new PrintWriter(ssh, Charset.defaultCharset().toString())) {
w.println("#!/bin/sh");
w.println("echo '" + quoteUnixCredentials(Secret.toString(sshUser.getPassphrase())) + "'");
Expand All @@ -1621,7 +1654,7 @@ private File createUnixSshAskpass(SSHUserPrivateKey sshUser) throws IOException

/* Package protected for testability */
File createWindowsBatFile(String userName, String password) throws IOException {
File askpass = File.createTempFile("pass", ".bat");
File askpass = createTempFile("pass", ".bat");
try (PrintWriter w = new PrintWriter(askpass, Charset.defaultCharset().toString())) {
w.println("@set arg=%~1");
w.println("@if (%arg:~0,8%)==(Username) echo " + escapeWindowsCharsForUnquotedString(userName));
Expand All @@ -1636,7 +1669,7 @@ private File createWindowsStandardAskpass(StandardUsernamePasswordCredentials cr
}

private File createUnixStandardAskpass(StandardUsernamePasswordCredentials creds) throws IOException {
File askpass = File.createTempFile("pass", ".sh");
File askpass = createTempFile("pass", ".sh");
try (PrintWriter w = new PrintWriter(askpass, Charset.defaultCharset().toString())) {
w.println("#!/bin/sh");
w.println("case \"$1\" in");
Expand Down Expand Up @@ -1766,7 +1799,7 @@ private File getSSHExeFromGitExeParentDir(String userGitExe) {
}

private File createWindowsGitSSH(File key, String user) throws IOException {
File ssh = File.createTempFile("ssh", ".bat");
File ssh = createTempFile("ssh", ".bat");

File sshexe = getSSHExecutable();

Expand All @@ -1779,7 +1812,7 @@ private File createWindowsGitSSH(File key, String user) throws IOException {
}

private File createUnixGitSSH(File key, String user) throws IOException {
File ssh = File.createTempFile("ssh", ".sh");
File ssh = createTempFile("ssh", ".sh");
try (PrintWriter w = new PrintWriter(ssh, Charset.defaultCharset().toString())) {
w.println("#!/bin/sh");
// ${SSH_ASKPASS} might be ignored if ${DISPLAY} is not set
Expand Down Expand Up @@ -2383,7 +2416,7 @@ public void branch(String name) throws GitException, InterruptedException {
public void commit(String message) throws GitException, InterruptedException {
File f = null;
try {
f = File.createTempFile("gitcommit", ".txt");
f = createTempFile("gitcommit", ".txt");
try (OutputStream out = Files.newOutputStream(f.toPath())) {
out.write(message.getBytes(Charset.defaultCharset().toString()));
}
Expand Down

0 comments on commit 75ea3fe

Please sign in to comment.