This is an automated email from the ASF dual-hosted git repository. dashorst pushed a commit to branch wicket-9.x in repository https://gitbox.apache.org/repos/asf/wicket.git
commit 713f77aa9ccfefac359e0fee4020fa3f6ff4eaed Author: reiern70 <[email protected]> AuthorDate: Wed Sep 30 13:06:22 2026 -0500 Sign file upload resource constraints FileUploadToResourceField now renders an encrypted token that covers the upload id and the maxSize, fileMaxSize and fileCountMax it was configured with, and AbstractFileUploadResource only accepts an upload whose token matches the values in the request. Client code that posts to the upload resource without the token rendered by the field now receives the uploadFailed error. --- .../resource/AbstractFileUploadResource.java | 84 +++++++- .../upload/resource/FileUploadToResourceField.java | 2 + .../upload/resource/FileUploadToResourceField.js | 9 +- ...loadToResourceFieldSecurityTest$UploadPage.html | 21 ++ .../FileUploadToResourceFieldSecurityTest.java | 217 +++++++++++++++++++++ 5 files changed, 326 insertions(+), 7 deletions(-) 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 8e4dee88a6..10d907064e 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; @@ -57,6 +59,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 @@ -78,6 +84,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 * @@ -98,6 +112,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); @@ -183,6 +203,64 @@ 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 = webRequest.getRequestParameters().getParameterValue(UPLOAD_TOKEN) + .toOptionalString(); + if (actualToken == null) + { + return false; + } + + // the token is authenticated but not deterministic - encryption uses a fresh nonce per + // message - so it cannot be validated by re-encrypting and comparing ciphertext. Decrypt + // it instead and compare the settings it carries; the authentication tag is what makes it + // unforgeable, and decryption fails on anything that was tampered with. + String decryptedToken = Application.get().getSecuritySettings().getCryptFactory().newCrypt() + .decryptUrlSafe(actualToken); + if (decryptedToken == null) + { + return false; + } + + String expectedSettings = serializeUploadSettings(uploadId, maxSize, fileMaxSize, + fileCountMax); + return MessageDigest.isEqual(expectedSettings.getBytes(StandardCharsets.UTF_8), + decryptedToken.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) { @@ -252,7 +330,7 @@ public abstract class AbstractFileUploadResource extends AbstractResource { try { - return Bytes.bytes(webRequest.getRequestParameters().getParameterValue("maxSize").toLong()); + return Bytes.bytes(webRequest.getRequestParameters().getParameterValue(MAX_SIZE).toLong()); } catch (StringValueConversionException e) { @@ -268,7 +346,7 @@ public abstract class AbstractFileUploadResource extends AbstractResource */ private Bytes getFileMaxSize(ServletWebRequest webRequest) { - long fileMaxSize = webRequest.getRequestParameters().getParameterValue("fileMaxSize").toLong(-1); + long fileMaxSize = webRequest.getRequestParameters().getParameterValue(FILE_MAX_SIZE).toLong(-1); return fileMaxSize > 0 ? Bytes.bytes(fileMaxSize) : null; } @@ -279,7 +357,7 @@ public abstract class AbstractFileUploadResource extends AbstractResource */ private long getFileCountMax(ServletWebRequest webRequest) { - return webRequest.getRequestParameters().getParameterValue("fileCountMax").toLong(-1); + return webRequest.getRequestParameters().getParameterValue(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 c76fb7939c..1666ec5684 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 @@ -422,6 +422,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 +})(); diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceFieldSecurityTest$UploadPage.html b/wicket-core/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceFieldSecurityTest$UploadPage.html new file mode 100644 index 0000000000..7b188cc87a --- /dev/null +++ b/wicket-core/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceFieldSecurityTest$UploadPage.html @@ -0,0 +1,21 @@ +<!-- + 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. +--> +<html xmlns:wicket="http://wicket.apache.org";> +<body> + <input type="file" wicket:id="upload"/> +</body> +</html> diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceFieldSecurityTest.java b/wicket-core/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceFieldSecurityTest.java new file mode 100644 index 0000000000..ec62950a7e --- /dev/null +++ b/wicket-core/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceFieldSecurityTest.java @@ -0,0 +1,217 @@ +/* + * 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.assertNotEquals; +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 javax.crypto.KeyGenerator; +import javax.crypto.SecretKey; +import org.apache.wicket.ajax.AjaxRequestTarget; +import org.apache.wicket.core.random.ISecureRandomSupplier; +import org.apache.wicket.core.util.crypt.AESCrypt; +import org.apache.wicket.markup.html.WebPage; +import org.apache.wicket.markup.html.form.upload.resource.FileUploadToResourceField.UploadInfo; +import org.apache.wicket.request.mapper.parameter.PageParameters; +import org.apache.wicket.util.crypt.ICrypt; +import org.apache.wicket.util.file.File; +import org.apache.wicket.util.lang.Bytes; +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 acceptsUploadWhenEncryptionIsNotDeterministic() throws Exception + { + SecretKey key = KeyGenerator.getInstance("AES").generateKey(); + ISecureRandomSupplier randomSupplier = tester.getApplication().getSecuritySettings() + .getRandomSupplier(); + tester.getApplication().getSecuritySettings() + .setCryptFactory(() -> new AESCrypt(key, randomSupplier)); + ICrypt crypt = tester.getApplication().getSecuritySettings().getCryptFactory().newCrypt(); + assertNotEquals(crypt.encryptUrlSafe("settings"), crypt.encryptUrlSafe("settings")); + + 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()); + } + + @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); + } + } +}
