This is an automated email from the ASF dual-hosted git repository. reiern70 pushed a commit to branch reiern70/upload-token-fix in repository https://gitbox.apache.org/repos/asf/wicket.git
commit 34ea50eb6361b4e30d925a87de9f27e8835d0802 Author: mundur <[email protected]> AuthorDate: Tue Jul 14 22:46:09 2026 +0800 Sign file upload resource constraints --- ...loadToResourceFieldSecurityTest$UploadPage.html | 5 + .../FileUploadToResourceFieldSecurityTest.java | 188 +++++++++++++++++++++ .../resource/AbstractFileUploadResource.java | 71 +++++++- .../upload/resource/FileUploadToResourceField.java | 2 + .../upload/resource/FileUploadToResourceField.js | 9 +- 5 files changed, 268 insertions(+), 7 deletions(-) diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceFieldSecurityTest$UploadPage.html b/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceFieldSecurityTest$UploadPage.html new file mode 100644 index 0000000000..2658b563d3 --- /dev/null +++ b/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceFieldSecurityTest$UploadPage.html @@ -0,0 +1,5 @@ +<html xmlns:wicket="http://wicket.apache.org";> +<body> + <input type="file" wicket:id="upload"/> +</body> +</html> diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceFieldSecurityTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceFieldSecurityTest.java new file mode 100644 index 0000000000..0251ba071a --- /dev/null +++ b/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceFieldSecurityTest.java @@ -0,0 +1,188 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.wicket.markup.html.form.upload.resource; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Comparator; +import java.util.List; +import org.apache.wicket.ajax.AjaxRequestTarget; +import org.apache.wicket.markup.html.WebPage; +import org.apache.wicket.request.mapper.parameter.PageParameters; +import org.apache.wicket.util.lang.Bytes; +import org.apache.wicket.util.file.File; +import org.apache.wicket.util.tester.WicketTestCase; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +class FileUploadToResourceFieldSecurityTest extends WicketTestCase +{ + private Path tempDir; + + @AfterEach + void tearDown() throws IOException + { + if (tempDir != null) + { + try (var paths = Files.walk(tempDir)) + { + paths.sorted(Comparator.reverseOrder()).forEach(path -> + { + try + { + Files.deleteIfExists(path); + } + catch (IOException e) + { + throw new RuntimeException(e); + } + }); + } + } + } + + @Test + void acceptsUploadWithRenderedLimits() throws Exception + { + FolderUploadsFileManager fileManager = newFileManager(); + UploadPage page = new UploadPage(new TestFileUploadResourceReference(fileManager)); + tester.startPage(page); + + File upload = createUploadFile("allowed.txt", 256); + submitUpload(page.field.buildUploadUrl(), upload); + + File storedFile = fileManager.getFile(page.field.getMarkupId(), upload.getName()); + assertTrue(storedFile.exists()); + assertEquals(upload.length(), storedFile.length()); + } + + @Test + void rejectsTamperedUploadLimits() throws Exception + { + FolderUploadsFileManager fileManager = newFileManager(); + UploadPage page = new UploadPage(new TestFileUploadResourceReference(fileManager)); + tester.startPage(page); + + File upload = createUploadFile("oversized.txt", 4096); + String validToken = AbstractFileUploadResource.createUploadToken(page.field.getMarkupId(), + page.field.getMaxSize(), page.field.getFileMaxSize(), page.field.getFileCountMax()); + String tamperedUrl = page.field.getResourceUrl() + '?' + + AbstractFileUploadResource.UPLOAD_ID + '=' + page.field.getMarkupId() + '&' + + AbstractFileUploadResource.MAX_SIZE + '=' + Bytes.kilobytes(64).bytes() + '&' + + AbstractFileUploadResource.FILE_COUNT_MAX + '=' + page.field.getFileCountMax() + '&' + + AbstractFileUploadResource.UPLOAD_TOKEN + '=' + validToken; + + submitUpload(tamperedUrl, upload); + + File storedFile = fileManager.getFile(page.field.getMarkupId(), upload.getName()); + assertFalse(storedFile.exists()); + assertTrue(tester.getLastResponseAsString().contains("uploadFailed")); + } + + private FolderUploadsFileManager newFileManager() throws IOException + { + tempDir = Files.createTempDirectory("wicket-upload-security"); + return new FolderUploadsFileManager(new File(tempDir.toFile())); + } + + private File createUploadFile(String fileName, int size) throws IOException + { + Path filePath = tempDir.resolve(fileName); + Files.write(filePath, new byte[size]); + return new File(filePath.toFile()); + } + + private void submitUpload(String url, File upload) + { + tester.getRequest().setMethod("POST"); + tester.getRequest().addFile(AbstractFileUploadResource.PARAM_NAME, upload, "text/plain"); + tester.executeUrl(url); + } + + static class UploadPage extends WebPage + { + private static final long serialVersionUID = 1L; + + final TestField field; + + UploadPage(TestFileUploadResourceReference resourceReference) + { + field = new TestField("upload", resourceReference); + field.setMaxSize(Bytes.kilobytes(1)); + add(field); + } + } + + static class TestField extends FileUploadToResourceField + { + private static final long serialVersionUID = 1L; + + private final TestFileUploadResourceReference resourceReference; + + TestField(String id, TestFileUploadResourceReference resourceReference) + { + super(id); + this.resourceReference = resourceReference; + } + + @Override + protected FileUploadResourceReference getFileUploadResourceReference() + { + return resourceReference; + } + + @Override + protected void onUploadSuccess(AjaxRequestTarget target, List<UploadInfo> fileInfos) + { + } + + String buildUploadUrl() + { + StringBuilder url = new StringBuilder(getResourceUrl()); + url.append('?').append(AbstractFileUploadResource.UPLOAD_ID).append('=') + .append(getMarkupId()); + url.append('&').append(AbstractFileUploadResource.MAX_SIZE).append('=') + .append(getMaxSize().bytes()); + url.append('&').append(AbstractFileUploadResource.FILE_COUNT_MAX).append('=') + .append(getFileCountMax()); + url.append('&').append(AbstractFileUploadResource.UPLOAD_TOKEN).append('=') + .append(AbstractFileUploadResource.createUploadToken(getMarkupId(), getMaxSize(), + getFileMaxSize(), getFileCountMax())); + return url.toString(); + } + + String getResourceUrl() + { + return urlFor(resourceReference, new PageParameters()).toString(); + } + } + + static class TestFileUploadResourceReference extends FileUploadResourceReference + { + private static final long serialVersionUID = 1L; + + TestFileUploadResourceReference(IUploadsFileManager uploadFileManager) + { + super(uploadFileManager); + } + } +} diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/AbstractFileUploadResource.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/AbstractFileUploadResource.java index 50cf91ff85..dea29a3494 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/AbstractFileUploadResource.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/AbstractFileUploadResource.java @@ -17,6 +17,8 @@ package org.apache.wicket.markup.html.form.upload.resource; import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -60,6 +62,10 @@ public abstract class AbstractFileUploadResource extends AbstractResource private static final Logger LOG = LoggerFactory.getLogger(AbstractFileUploadResource.class); public static final String PARAM_NAME = "WICKET-FILE-UPLOAD"; + public static final String MAX_SIZE = "maxSize"; + public static final String FILE_MAX_SIZE = "fileMaxSize"; + public static final String FILE_COUNT_MAX = "fileCountMax"; + public static final String UPLOAD_TOKEN = "uploadToken"; /** * This resource is usually an application singleton. Thus, client side pass @@ -81,6 +87,14 @@ public abstract class AbstractFileUploadResource extends AbstractResource this.fileManager = fileManager; } + static String createUploadToken(String uploadId, Bytes maxSize, Bytes fileMaxSize, + long fileCountMax) + { + String plainText = serializeUploadSettings(uploadId, maxSize, fileMaxSize, fileCountMax); + return Application.get().getSecuritySettings().getCryptFactory().newCrypt() + .encryptUrlSafe(plainText); + } + /** * Reads and stores the uploaded files * @@ -101,6 +115,12 @@ public abstract class AbstractFileUploadResource extends AbstractResource Bytes maxSize = getMaxSize(webRequest); Bytes fileMaxSize = getFileMaxSize(webRequest); long fileCountMax = getFileCountMax(webRequest); + if (!hasValidUploadToken(webRequest, uploadId, maxSize, fileMaxSize, fileCountMax)) + { + LOG.warn("Rejected upload for '{}' because the signed upload settings were invalid", + uploadId); + return createErrorResponse(resourceResponse, Form.UPLOAD_FAILED_RESOURCE_KEY); + } try { MultipartServletWebRequest multiPartRequest = webRequest.newMultipartWebRequest(maxSize, uploadId); @@ -186,6 +206,51 @@ public abstract class AbstractFileUploadResource extends AbstractResource return resourceResponse; } + private static String serializeUploadSettings(String uploadId, Bytes maxSize, Bytes fileMaxSize, + long fileCountMax) + { + return uploadId + '\n' + bytesValue(maxSize) + '\n' + bytesValue(fileMaxSize) + '\n' + + fileCountMax; + } + + private static String bytesValue(Bytes value) + { + return value != null ? Long.toString(value.bytes()) : ""; + } + + private boolean hasValidUploadToken(ServletWebRequest webRequest, String uploadId, Bytes maxSize, + Bytes fileMaxSize, long fileCountMax) + { + String actualToken = getParameterValue(webRequest, UPLOAD_TOKEN).toOptionalString(); + if (actualToken == null) + { + return false; + } + + String expectedToken = createUploadToken(uploadId, maxSize, fileMaxSize, fileCountMax); + return MessageDigest.isEqual(expectedToken.getBytes(StandardCharsets.UTF_8), + actualToken.getBytes(StandardCharsets.UTF_8)); + } + + private ResourceResponse createErrorResponse(ResourceResponse resourceResponse, String errorMessage) + { + resourceResponse.setContentType("application/json"); + resourceResponse.setStatusCode(HttpServletResponse.SC_OK); + JSONObject json = new JSONObject(); + json.put("error", true); + json.put("errorMessage", errorMessage); + String error = json.toString(); + resourceResponse.setWriteCallback(new WriteCallback() + { + @Override + public void writeData(Attributes attributes) throws IOException + { + attributes.getResponse().write(error); + } + }); + return resourceResponse; + } + protected String getFileUploadExceptionKey(final FileUploadException e) { @@ -263,7 +328,7 @@ public abstract class AbstractFileUploadResource extends AbstractResource { try { - return Bytes.bytes(getParameterValue(webRequest, "maxSize").toLong()); + return Bytes.bytes(getParameterValue(webRequest, MAX_SIZE).toLong()); } catch (StringValueConversionException e) { @@ -279,7 +344,7 @@ public abstract class AbstractFileUploadResource extends AbstractResource */ private Bytes getFileMaxSize(ServletWebRequest webRequest) { - long fileMaxSize = getParameterValue(webRequest, "fileMaxSize").toLong(-1); + long fileMaxSize = getParameterValue(webRequest, FILE_MAX_SIZE).toLong(-1); return fileMaxSize > 0 ? Bytes.bytes(fileMaxSize) : null; } @@ -300,7 +365,7 @@ public abstract class AbstractFileUploadResource extends AbstractResource */ private long getFileCountMax(ServletWebRequest webRequest) { - return getParameterValue(webRequest, "fileCountMax").toLong(-1); + return getParameterValue(webRequest, FILE_COUNT_MAX).toLong(-1); } /** diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceField.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceField.java index fbff4d91e0..47d10fae0e 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceField.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceField.java @@ -373,6 +373,8 @@ public abstract class FileUploadToResourceField extends FileUploadField jsonObject.put("fileMaxSize", fileMaxSize.bytes()); } jsonObject.put("fileCountMax", getFileCountMax()); + jsonObject.put("uploadToken", AbstractFileUploadResource.createUploadToken(getMarkupId(), + getMaxSize(), fileMaxSize, getFileCountMax())); response.render(OnDomReadyHeaderItem.forScript("Wicket.Timer." + getMarkupId() + " = new Wicket.FileUploadToResourceField(" + jsonObject + "," diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceField.js b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceField.js index 815e66912f..1ed5e0808b 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceField.js +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceField.js @@ -28,13 +28,14 @@ this.settings = settings; this.inputName = settings.inputName; this.input = document.getElementById(this.inputName); - this.resourceUrl = settings.resourceUrl + "?uploadId=" + this.inputName + "&maxSize=" + this.settings.maxSize; + this.resourceUrl = settings.resourceUrl + "?uploadId=" + encodeURIComponent(this.inputName) + "&maxSize=" + encodeURIComponent(this.settings.maxSize); if (this.settings.fileMaxSize != null) { - this.resourceUrl = this.resourceUrl + "&fileMaxSize=" + this.settings.fileMaxSize; + this.resourceUrl = this.resourceUrl + "&fileMaxSize=" + encodeURIComponent(this.settings.fileMaxSize); } if (this.settings.fileCountMax != null) { - this.resourceUrl = this.resourceUrl + "&fileCountMax=" + this.settings.fileCountMax; + this.resourceUrl = this.resourceUrl + "&fileCountMax=" + encodeURIComponent(this.settings.fileCountMax); } + this.resourceUrl = this.resourceUrl + "&uploadToken=" + encodeURIComponent(this.settings.uploadToken); this.ajaxCallBackUrl = settings.ajaxCallBackUrl; this.clientBeforeSendCallBack = clientBeforeSendCallBack; this.clientSideSuccessCallBack = clientSideSuccessCallBack; @@ -106,4 +107,4 @@ Wicket.Log.log("Too late to cancel upload for field '" + this.inputName + "': the upload has already finished."); } } -})(); \ No newline at end of file +})();
