mirror of
https://github.com/Alfresco/alfresco-transform-core.git
synced 2026-09-16 18:12:54 +00:00
ACS-11995 Fix CodeQL Issues (#1288)
This commit is contained in:
@@ -34,6 +34,9 @@ env:
|
||||
GIT_PASSWORD: ${{ secrets.BOT_GITHUB_TOKEN }}
|
||||
GITHUB_ACTIONS_DEPLOY_TIMEOUT: 120
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
|
||||
jobs:
|
||||
pre_commit:
|
||||
runs-on: ubuntu-latest
|
||||
|
||||
@@ -10,6 +10,9 @@ on:
|
||||
workflow_call:
|
||||
workflow_dispatch:
|
||||
|
||||
permissions:
|
||||
contents: write
|
||||
|
||||
jobs:
|
||||
scan-dependencies:
|
||||
runs-on: ubuntu-latest
|
||||
|
||||
@@ -5,10 +5,15 @@ on:
|
||||
branches:
|
||||
- precommit/**
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
|
||||
jobs:
|
||||
format-code:
|
||||
name: "Reformat code"
|
||||
runs-on: ubuntu-latest
|
||||
permissions:
|
||||
contents: write
|
||||
if: contains(github.event.head_commit.message, '[reformat code]')
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
|
||||
+34
-9
@@ -83,12 +83,14 @@ public class FileManager
|
||||
{
|
||||
filename = checkFilename(false, filename);
|
||||
LogEntry.setTarget(filename);
|
||||
return TempFileProvider.createTempFile("target_", "_" + filename);
|
||||
File created = TempFileProvider.createTempFile("target_", "_" + filename);
|
||||
return assertInsideTempDir(created);
|
||||
}
|
||||
|
||||
public static void deleteFile(final File file) throws Exception
|
||||
{
|
||||
if (!file.delete())
|
||||
final File safeFile = assertInsideTempDir(file);
|
||||
if (!safeFile.delete())
|
||||
{
|
||||
throw new Exception("Failed to delete file");
|
||||
}
|
||||
@@ -105,8 +107,8 @@ public class FileManager
|
||||
*/
|
||||
private static String checkFilename(boolean source, String filename)
|
||||
{
|
||||
filename = getFilename(filename);
|
||||
if (filename == null || filename.isEmpty())
|
||||
filename = filename == null ? "" : new File(filename).getName();
|
||||
if (filename.isEmpty())
|
||||
{
|
||||
String sourceOrTarget = source ? "source" : "target";
|
||||
HttpStatus statusCode = source ? BAD_REQUEST : INTERNAL_SERVER_ERROR;
|
||||
@@ -115,11 +117,32 @@ public class FileManager
|
||||
return filename;
|
||||
}
|
||||
|
||||
private static void save(MultipartFile multipartFile, File file)
|
||||
private static File assertInsideTempDir(File candidate)
|
||||
{
|
||||
try
|
||||
{
|
||||
Files.copy(multipartFile.getInputStream(), file.toPath(),
|
||||
File tempRoot = TempFileProvider.getTempDir();
|
||||
String candidateCanonical = candidate.getCanonicalPath();
|
||||
String parentCanonical = tempRoot.getCanonicalPath();
|
||||
if (!candidateCanonical.startsWith(parentCanonical + File.separator)
|
||||
&& !candidateCanonical.equals(parentCanonical))
|
||||
{
|
||||
throw new TransformException(INTERNAL_SERVER_ERROR, "Resolved file escapes the temp directory");
|
||||
}
|
||||
return new File(candidateCanonical);
|
||||
}
|
||||
catch (IOException e)
|
||||
{
|
||||
throw new TransformException(INTERNAL_SERVER_ERROR, "Unable to resolve canonical path", e);
|
||||
}
|
||||
}
|
||||
|
||||
private static void save(MultipartFile multipartFile, File file)
|
||||
{
|
||||
final File safeFile = assertInsideTempDir(file);
|
||||
try
|
||||
{
|
||||
Files.copy(multipartFile.getInputStream(), safeFile.toPath(),
|
||||
StandardCopyOption.REPLACE_EXISTING);
|
||||
}
|
||||
catch (IOException e)
|
||||
@@ -130,9 +153,10 @@ public class FileManager
|
||||
|
||||
public static void save(Resource body, File file)
|
||||
{
|
||||
final File safeFile = assertInsideTempDir(file);
|
||||
try
|
||||
{
|
||||
Files.copy(body.getInputStream(), file.toPath(), StandardCopyOption.REPLACE_EXISTING);
|
||||
Files.copy(body.getInputStream(), safeFile.toPath(), StandardCopyOption.REPLACE_EXISTING);
|
||||
}
|
||||
catch (IOException e)
|
||||
{
|
||||
@@ -142,9 +166,10 @@ public class FileManager
|
||||
|
||||
private static Resource load(File file)
|
||||
{
|
||||
final File safeFile = assertInsideTempDir(file);
|
||||
try
|
||||
{
|
||||
Resource resource = new UrlResource(file.toURI());
|
||||
Resource resource = new UrlResource(safeFile.toURI());
|
||||
if (resource.exists() || resource.isReadable())
|
||||
{
|
||||
return resource;
|
||||
@@ -222,7 +247,7 @@ public class FileManager
|
||||
String filename = multipartFile.getOriginalFilename();
|
||||
long size = multipartFile.getSize();
|
||||
filename = checkFilename(true, filename);
|
||||
File file = TempFileProvider.createTempFile("source_", "_" + filename);
|
||||
File file = assertInsideTempDir(TempFileProvider.createTempFile("source_", "_" + filename));
|
||||
request.setAttribute(SOURCE_FILE, file);
|
||||
save(multipartFile, file);
|
||||
LogEntry.setSource(filename, size);
|
||||
|
||||
+2
@@ -30,6 +30,7 @@ import static org.springframework.http.HttpStatus.BAD_REQUEST;
|
||||
import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR;
|
||||
|
||||
import static org.alfresco.transform.base.executors.RuntimeExec.ExecutionResult;
|
||||
import static org.alfresco.transform.base.fs.FileManager.assertWithinTempDir;
|
||||
|
||||
import java.io.File;
|
||||
import java.util.Map;
|
||||
@@ -48,6 +49,7 @@ public abstract class AbstractCommandExecutor implements CommandExecutor
|
||||
@Override
|
||||
public void run(Map<String, String> properties, File targetFile, Long timeout)
|
||||
{
|
||||
targetFile = assertWithinTempDir(targetFile);
|
||||
timeout = timeout != null && timeout > 0 ? timeout : 0;
|
||||
final ExecutionResult result = transformCommand.execute(properties, timeout);
|
||||
|
||||
|
||||
@@ -41,6 +41,8 @@ import java.io.IOException;
|
||||
import java.io.InputStream;
|
||||
import java.io.OutputStream;
|
||||
import java.net.MalformedURLException;
|
||||
import java.net.URI;
|
||||
import java.net.URISyntaxException;
|
||||
import java.net.URL;
|
||||
import java.nio.file.Files;
|
||||
import java.util.UUID;
|
||||
@@ -57,6 +59,7 @@ import org.alfresco.transform.base.logging.LogEntry;
|
||||
import org.alfresco.transform.common.ExtensionService;
|
||||
import org.alfresco.transform.exceptions.TransformException;
|
||||
|
||||
@SuppressWarnings("PMD.GodClass")
|
||||
public class FileManager
|
||||
{
|
||||
public static final String SOURCE_FILE = "sourceFile";
|
||||
@@ -65,6 +68,34 @@ public class FileManager
|
||||
private FileManager()
|
||||
{}
|
||||
|
||||
static File assertContained(File candidate, File parent)
|
||||
{
|
||||
try
|
||||
{
|
||||
String candidateCanonical = candidate.getCanonicalPath();
|
||||
String parentCanonical = parent.getCanonicalPath();
|
||||
if (!candidateCanonical.equals(parentCanonical)
|
||||
&& !candidateCanonical.startsWith(parentCanonical + File.separator))
|
||||
{
|
||||
throw new TransformException(BAD_REQUEST, "The resolved path escapes the temp directory");
|
||||
}
|
||||
return new File(candidateCanonical);
|
||||
}
|
||||
catch (IOException e)
|
||||
{
|
||||
throw new TransformException(BAD_REQUEST, "Unable to resolve canonical path", e);
|
||||
}
|
||||
}
|
||||
|
||||
public static File assertWithinTempDir(File file)
|
||||
{
|
||||
if (file == null)
|
||||
{
|
||||
return null;
|
||||
}
|
||||
return assertContained(file, new File(System.getProperty("java.io.tmpdir")));
|
||||
}
|
||||
|
||||
public static File createSourceFile(HttpServletRequest request, InputStream inputStream, String sourceMimetype, String sourceFileName)
|
||||
{
|
||||
try
|
||||
@@ -74,14 +105,15 @@ public class FileManager
|
||||
? TempFileProvider.createTempFile("source_", extension)
|
||||
: TempFileProvider.createFileWithinUUIDTempDir(sourceFileName);
|
||||
|
||||
Files.copy(inputStream, file.toPath(), REPLACE_EXISTING);
|
||||
File safeFile = assertContained(file, file.getParentFile());
|
||||
Files.copy(inputStream, safeFile.toPath(), REPLACE_EXISTING);
|
||||
|
||||
if (request != null)
|
||||
{
|
||||
request.setAttribute(SOURCE_FILE, file);
|
||||
request.setAttribute(SOURCE_FILE, safeFile);
|
||||
}
|
||||
LogEntry.setSource(file.getName(), file.length());
|
||||
return file;
|
||||
LogEntry.setSource(safeFile.getName(), safeFile.length());
|
||||
return safeFile;
|
||||
}
|
||||
catch (Exception e)
|
||||
{
|
||||
@@ -94,7 +126,8 @@ public class FileManager
|
||||
try
|
||||
{
|
||||
String extension = "." + ExtensionService.getExtensionForTargetMimetype(targetMimetype, sourceMimetype);
|
||||
File file = TempFileProvider.createTempFile("target_", extension);
|
||||
File raw = TempFileProvider.createTempFile("target_", extension);
|
||||
File file = assertContained(raw, raw.getParentFile());
|
||||
if (request != null)
|
||||
{
|
||||
request.setAttribute(TARGET_FILE, file);
|
||||
@@ -160,9 +193,26 @@ public class FileManager
|
||||
{
|
||||
try
|
||||
{
|
||||
return new URL(directUrl).openStream();
|
||||
URL url = new URL(directUrl);
|
||||
String protocol = url.getProtocol();
|
||||
if ("http".equalsIgnoreCase(protocol) || "https".equalsIgnoreCase(protocol))
|
||||
{
|
||||
String host = url.getHost();
|
||||
if (host == null || !host.matches("[A-Za-z0-9._\\-]+"))
|
||||
{
|
||||
throw new TransformException(BAD_REQUEST, "Direct Access Url host is not allowed.");
|
||||
}
|
||||
catch (IllegalArgumentException e)
|
||||
return new URI(protocol, null, host, url.getPort(),
|
||||
url.getPath(), url.getQuery(), null).toURL().openStream();
|
||||
}
|
||||
if ("file".equalsIgnoreCase(protocol))
|
||||
{
|
||||
File localFile = assertWithinTempDir(new File(url.toURI()));
|
||||
return Files.newInputStream(localFile.toPath());
|
||||
}
|
||||
throw new TransformException(BAD_REQUEST, "Direct Access Url protocol is not allowed.");
|
||||
}
|
||||
catch (URISyntaxException | IllegalArgumentException | MalformedURLException e)
|
||||
{
|
||||
throw new TransformException(BAD_REQUEST, "Direct Access Url is invalid.", e);
|
||||
}
|
||||
@@ -234,7 +284,12 @@ public class FileManager
|
||||
{
|
||||
throw new TransformException(INSUFFICIENT_STORAGE, "Failed to create temp directory: " + tempDir);
|
||||
}
|
||||
return new File(tempDir, sourceFileName);
|
||||
String baseName = new File(sourceFileName == null ? "" : sourceFileName).getName();
|
||||
if (baseName.isEmpty() || ".".equals(baseName) || "..".equals(baseName))
|
||||
{
|
||||
throw new TransformException(BAD_REQUEST, "The source filename is invalid");
|
||||
}
|
||||
return assertContained(new File(tempDir, baseName), tempDir);
|
||||
}
|
||||
|
||||
private static File getTempDir()
|
||||
|
||||
@@ -32,6 +32,7 @@ import static org.springframework.http.HttpStatus.BAD_REQUEST;
|
||||
import static org.springframework.http.HttpStatus.CREATED;
|
||||
import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR;
|
||||
|
||||
import static org.alfresco.transform.base.fs.FileManager.assertWithinTempDir;
|
||||
import static org.alfresco.transform.base.fs.FileManager.createAttachment;
|
||||
import static org.alfresco.transform.base.fs.FileManager.createTargetFile;
|
||||
import static org.alfresco.transform.base.fs.FileManager.getDirectAccessUrlInputStream;
|
||||
@@ -378,6 +379,7 @@ public class TransformHandler
|
||||
|
||||
private OutputStream getOutputStreamFromFile(File targetFile) throws IOException
|
||||
{
|
||||
targetFile = assertWithinTempDir(targetFile);
|
||||
return new BufferedOutputStream(new FileOutputStream(targetFile));
|
||||
}
|
||||
|
||||
|
||||
+4
@@ -29,6 +29,8 @@ package org.alfresco.transform.libreoffice.transformers;
|
||||
import static org.springframework.http.HttpStatus.BAD_REQUEST;
|
||||
import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR;
|
||||
|
||||
import static org.alfresco.transform.base.fs.FileManager.assertWithinTempDir;
|
||||
|
||||
import java.io.File;
|
||||
import java.io.IOException;
|
||||
import java.io.Serializable;
|
||||
@@ -137,6 +139,8 @@ public class LibreOfficeTransformer implements JavaExecutor, CustomTransformerFi
|
||||
public void transform(String sourceMimetype, String targetMimetype, Map<String, String> transformOptions,
|
||||
File sourceFile, File targetFile, TransformManager transformManager)
|
||||
{
|
||||
sourceFile = assertWithinTempDir(sourceFile);
|
||||
targetFile = assertWithinTempDir(targetFile);
|
||||
call(sourceFile, targetFile);
|
||||
}
|
||||
|
||||
|
||||
+2
-1
@@ -26,6 +26,7 @@
|
||||
*/
|
||||
package org.alfresco.transform.misc.metadataExtractors;
|
||||
|
||||
import static org.alfresco.transform.base.fs.FileManager.assertWithinTempDir;
|
||||
import static org.alfresco.transform.base.metadata.AbstractMetadataExtractorEmbedder.Type.EXTRACTOR;
|
||||
|
||||
import java.io.File;
|
||||
@@ -104,7 +105,7 @@ public class HtmlMetadataExtractor extends AbstractMetadataExtractorEmbedder
|
||||
|
||||
// This Extractor retries if the encoding needs to be changed, so we need to reread the source,
|
||||
// so cannot use the input stream provided, as it will get closed.
|
||||
final File sourceFile = transformManager.createSourceFile();
|
||||
final File sourceFile = assertWithinTempDir(transformManager.createSourceFile());
|
||||
|
||||
HTMLEditorKit.ParserCallback callback = new HTMLEditorKit.ParserCallback() {
|
||||
StringBuffer title = null;
|
||||
|
||||
+3
@@ -26,6 +26,7 @@
|
||||
*/
|
||||
package org.alfresco.transform.misc.transformers;
|
||||
|
||||
import static org.alfresco.transform.base.fs.FileManager.assertWithinTempDir;
|
||||
import static org.alfresco.transform.common.Mimetype.MIMETYPE_IMAGE_JPEG;
|
||||
|
||||
import java.io.BufferedInputStream;
|
||||
@@ -83,6 +84,8 @@ public class AppleIWorksContentTransformer implements CustomTransformerFileAdapt
|
||||
public void transform(String sourceMimetype, String targetMimetype, Map<String, String> transformOptions,
|
||||
File sourceFile, File targetFile, TransformManager transformManager)
|
||||
{
|
||||
sourceFile = assertWithinTempDir(sourceFile);
|
||||
targetFile = assertWithinTempDir(targetFile);
|
||||
logger.debug("Performing IWorks to jpeg transform with sourceMimetype={} targetMimetype={}",
|
||||
sourceMimetype, targetMimetype);
|
||||
|
||||
|
||||
+3
@@ -26,6 +26,7 @@
|
||||
*/
|
||||
package org.alfresco.transform.misc.transformers;
|
||||
|
||||
import static org.alfresco.transform.base.fs.FileManager.assertWithinTempDir;
|
||||
import static org.alfresco.transform.common.Mimetype.MIMETYPE_HTML;
|
||||
import static org.alfresco.transform.common.Mimetype.MIMETYPE_MULTIPART_ALTERNATIVE;
|
||||
import static org.alfresco.transform.common.Mimetype.MIMETYPE_TEXT_PLAIN;
|
||||
@@ -81,6 +82,8 @@ public class EMLTransformer implements CustomTransformerFileAdaptor
|
||||
public void transform(String sourceMimetype, String targetMimetype, Map<String, String> transformOptions,
|
||||
File sourceFile, File targetFile, TransformManager transformManager) throws Exception
|
||||
{
|
||||
sourceFile = assertWithinTempDir(sourceFile);
|
||||
targetFile = assertWithinTempDir(targetFile);
|
||||
logger.debug("Performing RFC822 to text transform.");
|
||||
// Use try with resource
|
||||
try (InputStream contentInputStream = new BufferedInputStream(
|
||||
|
||||
+4
-1
@@ -26,6 +26,7 @@
|
||||
*/
|
||||
package org.alfresco.transform.misc.transformers;
|
||||
|
||||
import static org.alfresco.transform.base.fs.FileManager.assertWithinTempDir;
|
||||
import static org.alfresco.transform.common.RequestParamMap.HTML_COLLAPSE;
|
||||
import static org.alfresco.transform.common.RequestParamMap.SOURCE_ENCODING;
|
||||
|
||||
@@ -89,8 +90,10 @@ public class HtmlParserContentTransformer implements CustomTransformerFileAdapto
|
||||
@Override
|
||||
public void transform(final String sourceMimetype, final String targetMimetype,
|
||||
final Map<String, String> transformOptions,
|
||||
final File sourceFile, final File targetFile, TransformManager transformManager) throws Exception
|
||||
File sourceFile, File targetFile, TransformManager transformManager) throws Exception
|
||||
{
|
||||
sourceFile = assertWithinTempDir(sourceFile);
|
||||
targetFile = assertWithinTempDir(targetFile);
|
||||
String sourceEncoding = transformOptions.get(SOURCE_ENCODING);
|
||||
checkEncodingParameter(sourceEncoding, SOURCE_ENCODING);
|
||||
boolean collapse;
|
||||
|
||||
+5
-1
@@ -26,6 +26,8 @@
|
||||
*/
|
||||
package org.alfresco.transform.misc.transformers;
|
||||
|
||||
import static org.alfresco.transform.base.fs.FileManager.assertWithinTempDir;
|
||||
|
||||
import java.io.File;
|
||||
import java.io.IOException;
|
||||
import java.io.InputStream;
|
||||
@@ -68,8 +70,10 @@ public class OOXMLThumbnailContentTransformer implements CustomTransformerFileAd
|
||||
|
||||
@Override
|
||||
public void transform(final String sourceMimetype, final String targetMimetype, final Map<String, String> parameters,
|
||||
final File sourceFile, final File targetFile, TransformManager transformManager) throws Exception
|
||||
File sourceFile, File targetFile, TransformManager transformManager) throws Exception
|
||||
{
|
||||
sourceFile = assertWithinTempDir(sourceFile);
|
||||
targetFile = assertWithinTempDir(targetFile);
|
||||
if (logger.isDebugEnabled())
|
||||
{
|
||||
logger.debug("Performing OOXML to jpeg transform with sourceMimetype=" + sourceMimetype
|
||||
|
||||
+4
-1
@@ -26,6 +26,7 @@
|
||||
*/
|
||||
package org.alfresco.transform.misc.transformers;
|
||||
|
||||
import static org.alfresco.transform.base.fs.FileManager.assertWithinTempDir;
|
||||
import static org.alfresco.transform.common.RequestParamMap.SOURCE_ENCODING;
|
||||
import static org.alfresco.transform.common.RequestParamMap.TARGET_ENCODING;
|
||||
|
||||
@@ -79,8 +80,10 @@ public class StringExtractingContentTransformer implements CustomTransformerFile
|
||||
*/
|
||||
@Override
|
||||
public void transform(final String sourceMimetype, final String targetMimetype, final Map<String, String> transformOptions,
|
||||
final File sourceFile, final File targetFile, TransformManager transformManager) throws Exception
|
||||
File sourceFile, File targetFile, TransformManager transformManager) throws Exception
|
||||
{
|
||||
sourceFile = assertWithinTempDir(sourceFile);
|
||||
targetFile = assertWithinTempDir(targetFile);
|
||||
String sourceEncoding = transformOptions.get(SOURCE_ENCODING);
|
||||
String targetEncoding = transformOptions.get(TARGET_ENCODING);
|
||||
|
||||
|
||||
+4
-1
@@ -26,6 +26,7 @@
|
||||
*/
|
||||
package org.alfresco.transform.misc.transformers;
|
||||
|
||||
import static org.alfresco.transform.base.fs.FileManager.assertWithinTempDir;
|
||||
import static org.alfresco.transform.common.RequestParamMap.PAGE_LIMIT;
|
||||
import static org.alfresco.transform.common.RequestParamMap.PDF_FONT;
|
||||
import static org.alfresco.transform.common.RequestParamMap.PDF_FONT_SIZE;
|
||||
@@ -149,8 +150,10 @@ public class TextToPdfContentTransformer implements CustomTransformerFileAdaptor
|
||||
|
||||
@Override
|
||||
public void transform(final String sourceMimetype, final String targetMimetype, final Map<String, String> transformOptions,
|
||||
final File sourceFile, final File targetFile, TransformManager transformManager) throws Exception
|
||||
File sourceFile, File targetFile, TransformManager transformManager) throws Exception
|
||||
{
|
||||
sourceFile = assertWithinTempDir(sourceFile);
|
||||
targetFile = assertWithinTempDir(targetFile);
|
||||
String sourceEncoding = transformOptions.get(SOURCE_ENCODING);
|
||||
String stringPageLimit = transformOptions.get(PAGE_LIMIT);
|
||||
int pageLimit = -1;
|
||||
|
||||
@@ -218,12 +218,17 @@
|
||||
<artifactId>commons-codec</artifactId>
|
||||
<version>1.17.2</version>
|
||||
</dependency>
|
||||
<!-- temporary security fix CVE-2026-5598 / CVE-2026-0636 -->
|
||||
<!-- temporary security fix CVE-2026-5598 / CVE-2026-0636 / CVE-2026-5588 -->
|
||||
<dependency>
|
||||
<groupId>org.bouncycastle</groupId>
|
||||
<artifactId>bcprov-jdk18on</artifactId>
|
||||
<version>${dependency.bcprov.version}</version>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>org.bouncycastle</groupId>
|
||||
<artifactId>bcpkix-jdk18on</artifactId>
|
||||
<version>${dependency.bcprov.version}</version>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>org.apache.tomcat.embed</groupId>
|
||||
<artifactId>tomcat-embed-core</artifactId>
|
||||
|
||||
Reference in New Issue
Block a user