Skip to content

Commit

Permalink
Merge pull request #1523 from fl4via/2.2.x-backport-fixes
Browse files Browse the repository at this point in the history 
[UNDERTOW-2305][UNDERTOW-2271][UNDERTOW-2072]  CVE-2023-3223 Backport bug fixes
  • Loading branch information
@fl4via
fl4via authored Oct 11, 2023
2 parents 9d702e6 + 01fa3ef commit 7ef0812
Show file tree
Hide file tree
Showing 5 changed files with 122 additions and 68 deletions.
2 changes: 1 addition & 1 deletion core/src/main/java/io/undertow/client/http2/Http2ClearClientProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ public static String createSettingsFrame(OptionMap options, ByteBufferPool buffe
if (options.contains(UndertowOptions.HTTP2_SETTINGS_MAX_HEADER_LIST_SIZE)) {
pushOption(currentBuffer, Http2Setting.SETTINGS_MAX_HEADER_LIST_SIZE, options.get(UndertowOptions.HTTP2_SETTINGS_MAX_HEADER_LIST_SIZE));
} else if(options.contains(UndertowOptions.MAX_HEADER_SIZE)) {
pushOption(currentBuffer, Http2Setting.SETTINGS_MAX_HEADER_LIST_SIZE, options.get(UndertowOptions.HTTP2_SETTINGS_MAX_HEADER_LIST_SIZE));
pushOption(currentBuffer, Http2Setting.SETTINGS_MAX_HEADER_LIST_SIZE, options.get(UndertowOptions.MAX_HEADER_SIZE));
}
currentBuffer.flip();
return FlexBase64.encodeStringURL(currentBuffer, false);
Expand Down
16 changes: 10 additions & 6 deletions core/src/main/java/io/undertow/server/DefaultByteBufferPool.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ public PooledByteBuffer allocate() {
buffer = queue.poll();
if (buffer != null) {
currentQueueLengthUpdater.decrementAndGet(this);
//buffer.clear();
}
}
if (buffer == null) {
Expand Down Expand Up @@ -256,20 +257,23 @@ private static class DefaultPooledBuffer implements PooledByteBuffer {

@Override
public ByteBuffer getBuffer() {
if(referenceCount == 0) {
final ByteBuffer tmp = this.buffer;
//UNDERTOW-2072
if (referenceCount == 0 || tmp == null) {
throw UndertowMessages.MESSAGES.bufferAlreadyFreed();
}
return buffer;
return tmp;
}

@Override
public void close() {
if(referenceCountUpdater.compareAndSet(this, 1, 0)) {
if(leakDetector != null) {
final ByteBuffer tmp = this.buffer;
if (referenceCountUpdater.compareAndSet(this, 1, 0)) {
this.buffer = null;
if (leakDetector != null) {
leakDetector.closed = true;
}
pool.freeInternal(buffer);
this.buffer = null;
pool.freeInternal(tmp);
}
}

Expand Down
127 changes: 70 additions & 57 deletions core/src/main/java/io/undertow/server/handlers/form/MultiPartParserDefinition.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.StandardOpenOption;
import java.nio.file.attribute.FileAttribute;
import java.util.ArrayList;
import java.util.Arrays;
Expand All @@ -61,7 +60,13 @@
*/
public class MultiPartParserDefinition implements FormParserFactory.ParserDefinition<MultiPartParserDefinition> {


public static final String MULTIPART_FORM_DATA = "multipart/form-data";
/**
* Proposed default MINSIZE as 16 KB for content in memory before persisting to disk if file content exceeds
* {@link #fileSizeThreshold} and the <i>filename</i> is not specified in the form.
*/
private static final long MINSIZE = Long.getLong("io.undertow.multipart.minsize", 0x4000);

private Executor executor;

Expand All @@ -73,6 +78,12 @@ public class MultiPartParserDefinition implements FormParserFactory.ParserDefini

private long fileSizeThreshold;

/**
* The threshold of form field size to persist to disk.
* It takes effect only for the form fields which do not have <i>filename</i> specified.
*/
private long fieldSizeThreshold = MINSIZE;

public MultiPartParserDefinition() {
tempFileLocation = Paths.get(System.getProperty("java.io.tmpdir"));
}
Expand All @@ -90,7 +101,7 @@ public FormDataParser create(final HttpServerExchange exchange) {
UndertowLogger.REQUEST_LOGGER.debugf("Could not find boundary in multipart request with ContentType: %s, multipart data will not be available", mimeType);
return null;
}
final MultiPartUploadHandler parser = new MultiPartUploadHandler(exchange, boundary, maxIndividualFileSize, fileSizeThreshold, defaultEncoding);
final MultiPartUploadHandler parser = new MultiPartUploadHandler(exchange, boundary, maxIndividualFileSize, fileSizeThreshold, defaultEncoding, fieldSizeThreshold);
exchange.addExchangeCompleteListener(new ExchangeCompletionListener() {
@Override
public void exchangeEvent(final HttpServerExchange exchange, final NextListener nextListener) {
Expand Down Expand Up @@ -119,6 +130,11 @@ public MultiPartParserDefinition setExecutor(final Executor executor) {
return this;
}

public MultiPartParserDefinition setFieldSizeThreshold(long fieldSizeThreshold) {
this.fieldSizeThreshold = fieldSizeThreshold;
return this;
}

public Path getTempFileLocation() {
return tempFileLocation;
}
Expand Down Expand Up @@ -167,13 +183,14 @@ private final class MultiPartUploadHandler implements FormDataParser, MultipartP
private HttpHandler handler;
private long currentFileSize;
private final MultipartParser.ParseState parser;
private final long fieldSizeThreshold;


private MultiPartUploadHandler(final HttpServerExchange exchange, final String boundary, final long maxIndividualFileSize, final long fileSizeThreshold, final String defaultEncoding) {
private MultiPartUploadHandler(final HttpServerExchange exchange, final String boundary, final long maxIndividualFileSize, final long fileSizeThreshold, final String defaultEncoding, final long fieldSizeThreshold) {
this.exchange = exchange;
this.maxIndividualFileSize = maxIndividualFileSize;
this.defaultEncoding = defaultEncoding;
this.fileSizeThreshold = fileSizeThreshold;
this.fieldSizeThreshold = fieldSizeThreshold;
this.data = new FormData(exchange.getConnection().getUndertowOptions().get(UndertowOptions.MAX_PARAMETERS, 1000));
String charset = defaultEncoding;
String contentType = exchange.getRequestHeaders().getFirst(Headers.CONTENT_TYPE);
Expand Down Expand Up @@ -251,56 +268,52 @@ public void beginPart(final HeaderMap headers) {
if (disposition.startsWith("form-data")) {
currentName = Headers.extractQuotedValueFromHeader(disposition, "name");
fileName = Headers.extractQuotedValueFromHeaderWithEncoding(disposition, "filename");
if (fileName != null && fileSizeThreshold == 0) {
try {
if (tempFileLocation != null) {
//Files impl is buggy, hence zero len
final FileAttribute[] emptyFA = new FileAttribute[] {};
final LinkOption[] emptyLO = new LinkOption[] {};
final Path normalized = tempFileLocation.normalize();
if (!Files.exists(normalized)) {
final int pathElementsCount = normalized.getNameCount();
Path tmp = normalized;
LinkedList<Path> dirsToGuard = new LinkedList<>();
for(int i=0;i<pathElementsCount;i++) {
if(Files.exists(tmp, emptyLO)) {
if(!Files.isDirectory(tmp, emptyLO)) {
//First existing element in path is a file,
//this will cause java.nio.file.FileSystemException
throw UndertowMessages.MESSAGES.pathElementIsRegularFile(tmp);
}
break;
} else {
dirsToGuard.addFirst(tmp);
tmp = tmp.getParent();
}
}
try {
Files.createDirectories(normalized, emptyFA);
} finally {
for (Path p : dirsToGuard) {
try {
p.toFile().deleteOnExit();
} catch (Exception e) {
break;
}
}
}
} else if (!Files.isDirectory(normalized, emptyLO)) {
throw new IOException(UndertowMessages.MESSAGES.pathNotADirectory(normalized));
}
file = Files.createTempFile(normalized, "undertow", "upload");
} else {
file = Files.createTempFile("undertow", "upload");
}
}
}

private Path createFile() throws IOException {
if (tempFileLocation != null) {
//Files impl is buggy, hence zero len
final FileAttribute[] emptyFA = new FileAttribute[] {};
final LinkOption[] emptyLO = new LinkOption[] {};
final Path normalized = tempFileLocation.normalize();
if (!Files.exists(normalized)) {
final int pathElementsCount = normalized.getNameCount();
Path tmp = normalized;
LinkedList<Path> dirsToGuard = new LinkedList<>();
for(int i=0;i<pathElementsCount;i++) {
if(Files.exists(tmp, emptyLO)) {
if(!Files.isDirectory(tmp, emptyLO)) {
//First existing element in path is a file,
//this will cause java.nio.file.FileSystemException
throw UndertowMessages.MESSAGES.pathElementIsRegularFile(tmp);
}
break;
} else {
dirsToGuard.addFirst(tmp);
tmp = tmp.getParent();
}
}
try {
Files.createDirectories(normalized, emptyFA);
} finally {
for (Path p : dirsToGuard) {
try {
p.toFile().deleteOnExit();
} catch (Exception e) {
break;
}
createdFiles.add(file);
fileChannel = FileChannel.open(file, StandardOpenOption.READ, StandardOpenOption.WRITE);
} catch (IOException e) {
throw new RuntimeException(e);
}
}
} else if (!Files.isDirectory(normalized, emptyLO)) {
throw new IOException(UndertowMessages.MESSAGES.pathNotADirectory(normalized));
}
file = Files.createTempFile(normalized, "undertow", "upload");
} else {
file = Files.createTempFile("undertow", "upload");
}
return file;
}

@Override
Expand All @@ -309,14 +322,9 @@ public void data(final ByteBuffer buffer) throws IOException {
if (this.maxIndividualFileSize > 0 && this.currentFileSize > this.maxIndividualFileSize) {
throw UndertowMessages.MESSAGES.maxFileSizeExceeded(this.maxIndividualFileSize);
}
if (file == null && fileName != null && fileSizeThreshold < this.currentFileSize) {
if (file == null && fileSizeThreshold < this.currentFileSize && (fileName != null || this.currentFileSize > fieldSizeThreshold)) {
try {
if (tempFileLocation != null) {
file = Files.createTempFile(tempFileLocation, "undertow", "upload");
} else {
file = Files.createTempFile("undertow", "upload");
}
createdFiles.add(file);
createdFiles.add(createFile());

FileOutputStream fileOutputStream = new FileOutputStream(file.toFile());
contentBytes.writeTo(fileOutputStream);
Expand All @@ -339,7 +347,12 @@ public void data(final ByteBuffer buffer) throws IOException {
@Override
public void endPart() {
if (file != null) {
data.add(currentName, file, fileName, headers);
if (fileName != null) {
data.add(currentName, file, fileName, headers);
} else {
final Path fileNamePath = file.getFileName();
data.add(currentName, file, fileNamePath != null ? fileNamePath.toString() : "", headers);
}
file = null;
contentBytes.reset();
try {
Expand Down
11 changes: 7 additions & 4 deletions core/src/main/java/io/undertow/server/session/InMemorySessionManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,15 +228,18 @@ private String createAndSaveNewID(SessionImpl session) {
public Session getSession(final HttpServerExchange serverExchange, final SessionConfig config) {
if (serverExchange != null) {
SessionImpl newSession = serverExchange.getAttachment(NEW_SESSION);
if(newSession != null) {
if (newSession != null) {
return newSession;
}
} else {
return null;
}

if (config == null) {
throw UndertowMessages.MESSAGES.couldNotFindSessionCookieConfig();
}

String sessionId = config.findSessionId(serverExchange);
InMemorySessionManager.SessionImpl session = (SessionImpl) getSession(sessionId);
if(session != null && serverExchange != null) {
if (session != null && serverExchange != null) {
session.requestStarted(serverExchange);
}
return session;
Expand Down
34 changes: 34 additions & 0 deletions core/src/test/java/io/undertow/server/handlers/form/MultipartFormDataParserTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,15 @@
import org.apache.commons.codec.digest.DigestUtils;
import org.apache.commons.io.Charsets;
import org.apache.commons.io.IOUtils;
import org.apache.http.HttpEntity;
import org.apache.http.HttpResponse;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.entity.ContentType;
import org.apache.http.entity.StringEntity;
import org.apache.http.entity.mime.FormBodyPart;
import org.apache.http.entity.mime.HttpMultipartMode;
import org.apache.http.entity.mime.MultipartEntity;
import org.apache.http.entity.mime.MultipartEntityBuilder;
import org.apache.http.entity.mime.content.FileBody;
import org.apache.http.entity.mime.content.StringBody;
import org.junit.Assert;
Expand Down Expand Up @@ -345,6 +348,37 @@ public void testFileUploadWithMediumFileSizeThresholdAndLargeFile() throws Excep
}
}

@Test
public void testLargeContentWithoutFileNameWithSmallFileSizeThreshold() throws Exception {
DefaultServer.setRootHandler(new BlockingHandler(createInMemoryReadingHandler(10)));
File file = new File("tmp_upload_file.txt");
try (TestHttpClient client = new TestHttpClient()) {
file.createNewFile();
// 32 kb content, which exceeds default fieldSizeThreshold, the FormData.FormValue will be a FileItem
writeLargeFileContent(file, 0x4000 * 2);
HttpPost post = new HttpPost(DefaultServer.getDefaultServerURL() + "/path");
HttpEntity entity = MultipartEntityBuilder.create()
.addTextBody("formValue", "myValue", ContentType.TEXT_PLAIN)
.addBinaryBody("file", Files.newInputStream(file.toPath()))
.build();
post.setEntity(entity);
HttpResponse result = client.execute(post);
Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode());
String resp = HttpClientUtils.readResponse(result);

Map<String, String> parsedResponse = parse(resp);

Assert.assertEquals("false", parsedResponse.get("in_memory"));
Assert.assertEquals(DigestUtils.md5Hex(Files.newInputStream(file.toPath())), parsedResponse.get("hash"));
Assert.assertTrue(parsedResponse.get("file_name").startsWith("undertow"));
Assert.assertTrue(parsedResponse.get("file_name").endsWith("upload"));
} finally {
if (!file.delete()) {
file.deleteOnExit();
}
}
}

private void writeLargeFileContent(File file, int size) throws IOException {
int textLength = "content".getBytes().length;
FileOutputStream fos = new FileOutputStream(file);
Expand Down

0 comments on commit 7ef0812

Please sign in to comment.