Skip to content
Permalink
Browse files Browse the repository at this point in the history
Use Files.createTempFile(...) to ensure the file is created with prop…
…er permissions

Motivation:

File.createTempFile(String, String)` will create a temporary file in the system temporary directory if the 'java.io.tmpdir'. The permissions on that file utilize the umask. In a majority of cases, this means that the file that java creates has the permissions: `-rw-r--r--`, thus, any other local user on that system can read the contents of that file.
This can be a security concern if any sensitive data is stored in this file.

This was reported by Jonathan Leitschuh <jonathan.leitschuh@gmail.com> as a security problem.

Modifications:

Use Files.createTempFile(...) which will use safe-defaults when running on java 7 and later. If running on java 6 there isnt much we can do, which is fair enough as java 6 shouldnt be considered "safe" anyway.

Result:

Create temporary files with sane permissions by default.
  • Loading branch information
normanmaurer committed Feb 8, 2021
1 parent 8f397e2 commit c735357
Show file tree
Hide file tree
Showing 16 changed files with 47 additions and 20 deletions.
4 changes: 2 additions & 2 deletions buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java
Expand Up @@ -4551,7 +4551,7 @@ private void testGetReadOnlyDst(boolean direct) {

@Test
public void testReadBytesAndWriteBytesWithFileChannel() throws IOException {
File file = File.createTempFile("file-channel", ".tmp");
File file = PlatformDependent.createTempFile("file-channel", ".tmp", null);
RandomAccessFile randomAccessFile = null;
try {
randomAccessFile = new RandomAccessFile(file, "rw");
Expand Down Expand Up @@ -4594,7 +4594,7 @@ public void testReadBytesAndWriteBytesWithFileChannel() throws IOException {

@Test
public void testGetBytesAndSetBytesWithFileChannel() throws IOException {
File file = File.createTempFile("file-channel", ".tmp");
File file = PlatformDependent.createTempFile("file-channel", ".tmp", null);
RandomAccessFile randomAccessFile = null;
try {
randomAccessFile = new RandomAccessFile(file, "rw");
Expand Down
Expand Up @@ -306,7 +306,7 @@ public void testWrapBufferRoundTrip() {

@Test
public void testWrapMemoryMapped() throws Exception {
File file = File.createTempFile("netty-test", "tmp");
File file = PlatformDependent.createTempFile("netty-test", "tmp", null);
FileChannel output = null;
FileChannel input = null;
ByteBuf b1 = null;
Expand Down
Expand Up @@ -19,6 +19,7 @@
import io.netty.handler.codec.http.HttpConstants;
import io.netty.util.internal.EmptyArrays;
import io.netty.util.internal.ObjectUtil;
import io.netty.util.internal.PlatformDependent;
import io.netty.util.internal.logging.InternalLogger;
import io.netty.util.internal.logging.InternalLoggerFactory;

Expand Down Expand Up @@ -88,9 +89,9 @@ private File tempFile() throws IOException {
File tmpFile;
if (getBaseDirectory() == null) {
// create a temporary file
tmpFile = File.createTempFile(getPrefix(), newpostfix);
tmpFile = PlatformDependent.createTempFile(getPrefix(), newpostfix, null);
} else {
tmpFile = File.createTempFile(getPrefix(), newpostfix, new File(
tmpFile = PlatformDependent.createTempFile(getPrefix(), newpostfix, new File(
getBaseDirectory()));
}
if (deleteOnExit()) {
Expand Down
Expand Up @@ -25,6 +25,7 @@
import io.netty.handler.stream.ChunkedNioStream;
import io.netty.handler.stream.ChunkedStream;
import io.netty.handler.stream.ChunkedWriteHandler;
import io.netty.util.internal.PlatformDependent;
import org.junit.Test;

import java.io.ByteArrayInputStream;
Expand All @@ -46,7 +47,7 @@ public class HttpChunkedInputTest {

FileOutputStream out = null;
try {
TMP = File.createTempFile("netty-chunk-", ".tmp");
TMP = PlatformDependent.createTempFile("netty-chunk-", ".tmp", null);
TMP.deleteOnExit();
out = new FileOutputStream(TMP);
out.write(BYTES);
Expand Down
Expand Up @@ -39,7 +39,7 @@ public class AbstractDiskHttpDataTest {
public void testGetChunk() throws Exception {
TestHttpData test = new TestHttpData("test", UTF_8, 0);
try {
File tmpFile = File.createTempFile(UUID.randomUUID().toString(), ".tmp");
File tmpFile = PlatformDependent.createTempFile(UUID.randomUUID().toString(), ".tmp", null);
tmpFile.deleteOnExit();
FileOutputStream fos = new FileOutputStream(tmpFile);
byte[] bytes = new byte[4096];
Expand Down
Expand Up @@ -43,7 +43,7 @@ public class AbstractMemoryHttpDataTest {
public void testSetContentFromFile() throws Exception {
TestHttpData test = new TestHttpData("test", UTF_8, 0);
try {
File tmpFile = File.createTempFile(UUID.randomUUID().toString(), ".tmp");
File tmpFile = PlatformDependent.createTempFile(UUID.randomUUID().toString(), ".tmp", null);
tmpFile.deleteOnExit();
FileOutputStream fos = new FileOutputStream(tmpFile);
byte[] bytes = new byte[4096];
Expand All @@ -70,7 +70,7 @@ public void testSetContentFromFile() throws Exception {
public void testRenameTo() throws Exception {
TestHttpData test = new TestHttpData("test", UTF_8, 0);
try {
File tmpFile = File.createTempFile(UUID.randomUUID().toString(), ".tmp");
File tmpFile = PlatformDependent.createTempFile(UUID.randomUUID().toString(), ".tmp", null);
tmpFile.deleteOnExit();
final int totalByteCount = 4096;
byte[] bytes = new byte[totalByteCount];
Expand Down
Expand Up @@ -273,7 +273,7 @@ public void setSetContentFromFileExceptionally() throws Exception {
assertEquals(maxSize, f1.length());
byte[] bytes = new byte[8];
PlatformDependent.threadLocalRandom().nextBytes(bytes);
File tmpFile = File.createTempFile(UUID.randomUUID().toString(), ".tmp");
File tmpFile = PlatformDependent.createTempFile(UUID.randomUUID().toString(), ".tmp", null);
tmpFile.deleteOnExit();
FileOutputStream fos = new FileOutputStream(tmpFile);
try {
Expand Down
Expand Up @@ -177,7 +177,7 @@ public static void load(String originalName, ClassLoader loader) {
String prefix = libname.substring(0, index);
String suffix = libname.substring(index);

tmpFile = File.createTempFile(prefix, suffix, WORKDIR);
tmpFile = PlatformDependent.createTempFile(prefix, suffix, WORKDIR);
in = url.openStream();
out = new FileOutputStream(tmpFile);

Expand Down
19 changes: 19 additions & 0 deletions common/src/main/java/io/netty/util/internal/PlatformDependent.java
Expand Up @@ -38,6 +38,7 @@
import java.lang.reflect.Method;
import java.nio.ByteBuffer;
import java.nio.ByteOrder;
import java.nio.file.Files;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.util.Arrays;
Expand Down Expand Up @@ -1389,6 +1390,24 @@ public static Set<String> normalizedLinuxClassifiers() {
return LINUX_OS_CLASSIFIERS;
}

@SuppressJava6Requirement(reason = "Guarded by version check")
public static File createTempFile(String prefix, String suffix, File directory) throws IOException {
if (javaVersion() >= 7) {
if (directory == null) {
return Files.createTempFile(prefix, suffix).toFile();
}
return Files.createTempFile(directory.toPath(), prefix, suffix).toFile();
}
if (directory == null) {
return File.createTempFile(prefix, suffix);
}
File file = File.createTempFile(prefix, suffix, directory);
// Try to adjust the perms, if this fails there is not much else we can do...
file.setReadable(false, false);
file.setReadable(true, true);
return file;
}

/**
* Adds only those classifier strings to <tt>dest</tt> which are present in <tt>allowed</tt>.
*
Expand Down
Expand Up @@ -20,6 +20,7 @@
import io.netty.buffer.Unpooled;
import io.netty.handler.codec.base64.Base64;
import io.netty.util.CharsetUtil;
import io.netty.util.internal.PlatformDependent;
import io.netty.util.internal.SystemPropertyUtil;
import io.netty.util.internal.ThrowableUtil;
import io.netty.util.internal.logging.InternalLogger;
Expand All @@ -30,6 +31,7 @@
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.nio.file.Files;
import java.security.KeyPair;
import java.security.KeyPairGenerator;
import java.security.NoSuchAlgorithmException;
Expand Down Expand Up @@ -330,7 +332,7 @@ static String[] newSelfSignedCertificate(
wrappedBuf.release();
}

File keyFile = File.createTempFile("keyutil_" + fqdn + '_', ".key");
File keyFile = PlatformDependent.createTempFile("keyutil_" + fqdn + '_', ".key", null);
keyFile.deleteOnExit();

OutputStream keyOut = new FileOutputStream(keyFile);
Expand Down Expand Up @@ -361,7 +363,7 @@ static String[] newSelfSignedCertificate(
wrappedBuf.release();
}

File certFile = File.createTempFile("keyutil_" + fqdn + '_', ".crt");
File certFile = PlatformDependent.createTempFile("keyutil_" + fqdn + '_', ".crt", null);
certFile.deleteOnExit();

OutputStream certOut = new FileOutputStream(certFile);
Expand Down
Expand Up @@ -26,6 +26,7 @@
import io.netty.channel.embedded.EmbeddedChannel;
import io.netty.util.CharsetUtil;
import io.netty.util.ReferenceCountUtil;
import io.netty.util.internal.PlatformDependent;
import org.junit.Assert;
import org.junit.Test;

Expand Down Expand Up @@ -55,7 +56,7 @@ public class ChunkedWriteHandlerTest {

FileOutputStream out = null;
try {
TMP = File.createTempFile("netty-chunk-", ".tmp");
TMP = PlatformDependent.createTempFile("netty-chunk-", ".tmp", null);
TMP.deleteOnExit();
out = new FileOutputStream(TMP);
out.write(BYTES);
Expand Down
Expand Up @@ -32,6 +32,7 @@
import io.netty.channel.socket.nio.NioSocketChannel;
import io.netty.handler.codec.LineBasedFrameDecoder;
import io.netty.util.CharsetUtil;
import io.netty.util.internal.PlatformDependent;
import org.junit.After;
import org.junit.Before;
import org.junit.BeforeClass;
Expand Down Expand Up @@ -61,7 +62,7 @@ public static void beforeClass() throws IOException {
BYTES[i] = (byte) r.nextInt(255);
}

tmp = File.createTempFile("netty-traffic", ".tmp");
tmp = PlatformDependent.createTempFile("netty-traffic", ".tmp", null);
tmp.deleteOnExit();
FileOutputStream out = null;
try {
Expand Down
Expand Up @@ -102,7 +102,7 @@ public void testFileRegionVoidPromiseNotAutoRead(ServerBootstrap sb, Bootstrap c
}

public void testFileRegionCountLargerThenFile(ServerBootstrap sb, Bootstrap cb) throws Throwable {
File file = File.createTempFile("netty-", ".tmp");
File file = PlatformDependent.createTempFile("netty-", ".tmp", null);
file.deleteOnExit();

final FileOutputStream out = new FileOutputStream(file);
Expand Down Expand Up @@ -136,7 +136,7 @@ private static void testFileRegion0(
cb.option(ChannelOption.AUTO_READ, autoRead);

final int bufferSize = 1024;
final File file = File.createTempFile("netty-", ".tmp");
final File file = PlatformDependent.createTempFile("netty-", ".tmp", null);
file.deleteOnExit();

final FileOutputStream out = new FileOutputStream(file);
Expand Down
Expand Up @@ -28,6 +28,7 @@
import io.netty.channel.SimpleChannelInboundHandler;
import io.netty.channel.unix.FileDescriptor;
import io.netty.util.NetUtil;
import io.netty.util.internal.PlatformDependent;
import org.junit.Assert;
import org.junit.Test;

Expand Down Expand Up @@ -192,7 +193,7 @@ public void operationComplete(ChannelFuture future) throws Exception {
@Test(timeout = 10000)
public void spliceToFile() throws Throwable {
EventLoopGroup group = new EpollEventLoopGroup(1);
File file = File.createTempFile("netty-splice", null);
File file = PlatformDependent.createTempFile("netty-splice", null, null);
file.deleteOnExit();

SpliceHandler sh = new SpliceHandler(file);
Expand Down
Expand Up @@ -17,6 +17,7 @@

import io.netty.channel.unix.DomainSocketAddress;
import io.netty.channel.unix.Socket;
import io.netty.util.internal.PlatformDependent;

import java.io.File;
import java.io.IOException;
Expand All @@ -26,7 +27,7 @@ public static DomainSocketAddress newSocketAddress() {
try {
File file;
do {
file = File.createTempFile("NETTY", "UDS");
file = PlatformDependent.createTempFile("NETTY", "UDS", null);
if (!file.delete()) {
throw new IOException("failed to delete: " + file);
}
Expand Down
Expand Up @@ -39,7 +39,7 @@ public class DefaultFileRegionTest {
}

private static File newFile() throws IOException {
File file = File.createTempFile("netty-", ".tmp");
File file = PlatformDependent.createTempFile("netty-", ".tmp", null);
file.deleteOnExit();

final FileOutputStream out = new FileOutputStream(file);
Expand Down

0 comments on commit c735357

Please sign in to comment.