fix: address CodeQL findings (zip-slip + implicit narrowing) (#7576)

* DataInstaller: reject archive entries that escape the install dir

CodeQL java/zipslip flagged populateFileAndDirLists at lines 750/753;
the actual sink is createFiles at line 619 (FileOutputStream) and
createDirectories at line 617 (mkdirs), reached via a string-concat
correctFileName that didn't normalise '..' segments.

correctFileName now resolves each entry against its base directory and
rejects any path whose canonical form leaves that base. Both
canonicalisations happen on the resolved File, so symlinks, '.' and
'..' segments are all collapsed before comparison. The two callers
that didn't already throw IOException (checkOverwriteOK and
createDirectories) catch and abort the install with the existing
error-dialog pattern.

A hostile data set containing 'data/../../etc/whatever' would now be
refused before any file is written.

* SkillModifier: make double-to-int truncation explicit

CodeQL java/implicit-cast-in-compound-assignment flagged 12 sites in
this file where a 'double' bonus was accumulated into an 'int' via +=,
which silently inserts (int) at every addition. The intent was always
truncating accumulation -- the function returns Integer and uses
.intValue() for the formula path -- so the warning is purely about
making the cast visible.

Fix: write the cast at every site. No behaviour change; the bytecode
already contained the same i2d/d2i pair.

* DataInstaller: regression test for zip-slip rejection

Reflective unit test so the same class compiles against both the
pre-fix (`private`, no checked exception) and post-fix
(package-private, throws IOException) signatures of correctFileName.
Verified by hand: temporarily reverting DataInstaller to master and
re-running this test yields 1 pass + 2 failures (both '..'-escape
cases are silently accepted under the old logic), and restoring the
fix flips it to 3 passes -- which makes the test a real regression
guard rather than a tautology.

correctFileName goes from `private` to package-private to give the
test in the same package direct access; reflection was needed only
for the cross-state run.

Author: Vest

code/src/java/pcgen/core/analysis/SkillModifier.java

@@ -52,48 +52,48 @@ public static Integer modifier(Skill sk, PlayerCharacter aPC)
 		{
 			PCStat stat = statref.get();
 			bonus = aPC.getStatModFor(stat);
-			bonus += aPC.getTotalBonusTo("SKILL", "STAT." + stat.getKeyName());
+			bonus += (int) aPC.getTotalBonusTo("SKILL", "STAT." + stat.getKeyName());
 		}
-		bonus += aPC.getTotalBonusTo("SKILL", keyName);
+		bonus += (int) aPC.getTotalBonusTo("SKILL", keyName);
 
 		// loop through all current skill types checking for boni
 		for (Type singleType : sk.getTrueTypeList(false))
 		{
-			bonus += aPC.getTotalBonusTo("SKILL", "TYPE." + singleType);
+			bonus += (int) aPC.getTotalBonusTo("SKILL", "TYPE." + singleType);
 		}
 
 		// now check for any lists of skills, etc
-		bonus += aPC.getTotalBonusTo("SKILL", "LIST");
+		bonus += (int) aPC.getTotalBonusTo("SKILL", "LIST");
 
 		// now check for ALL
-		bonus += aPC.getTotalBonusTo("SKILL", "ALL");
+		bonus += (int) aPC.getTotalBonusTo("SKILL", "ALL");
 
 		// these next two if-blocks try to get BONUS:[C]CSKILL|TYPE=xxx|y to
 		// function
 		if (aPC.isClassSkill(sk))
 		{
-			bonus += aPC.getTotalBonusTo("CSKILL", keyName);
+			bonus += (int) aPC.getTotalBonusTo("CSKILL", keyName);
 
 			// loop through all current skill types checking for boni
 			for (Type singleType : sk.getTrueTypeList(false))
 			{
-				bonus += aPC.getTotalBonusTo("CSKILL", "TYPE." + singleType);
+				bonus += (int) aPC.getTotalBonusTo("CSKILL", "TYPE." + singleType);
 			}
 
-			bonus += aPC.getTotalBonusTo("CSKILL", "LIST");
+			bonus += (int) aPC.getTotalBonusTo("CSKILL", "LIST");
 		}
 
 		if (!aPC.isClassSkill(sk) && !sk.getSafe(ObjectKey.EXCLUSIVE))
 		{
-			bonus += aPC.getTotalBonusTo("CCSKILL", keyName);
+			bonus += (int) aPC.getTotalBonusTo("CCSKILL", keyName);
 
 			// loop through all current skill types checking for boni
 			for (Type singleType : sk.getTrueTypeList(false))
 			{
-				bonus += aPC.getTotalBonusTo("CCSKILL", "TYPE." + singleType);
+				bonus += (int) aPC.getTotalBonusTo("CCSKILL", "TYPE." + singleType);
 			}
 
-			bonus += aPC.getTotalBonusTo("CCSKILL", "LIST");
+			bonus += (int) aPC.getTotalBonusTo("CCSKILL", "LIST");
 		}
 
 		// the above two if-blocks try to get
@@ -129,7 +129,7 @@ public static int getStatMod(Skill sk, PlayerCharacter pc)
 				SkillInfoUtilities.getKeyStatList(pc, sk, typeList);
                 for (Type type : typeList)
                 {
-                    statMod += pc.getTotalBonusTo("SKILL", "TYPE." + type);
+                    statMod += (int) pc.getTotalBonusTo("SKILL", "TYPE." + type);
                 }
 			}
 			return statMod;

code/src/java/pcgen/gui2/dialog/DataInstaller.java

@@ -29,6 +29,7 @@
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.nio.file.Path;
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.OutputStream;
@@ -488,7 +489,19 @@ private static boolean checkOverwriteOK(Collection<String> files, File destDir)
         Collection<String> existingFilesCorr = new ArrayList<>();
         for (String filename : files)
         {
-            String correctedFilename = correctFileName(destDir, filename);
+            String correctedFilename;
+            try
+            {
+                correctedFilename = correctFileName(destDir, filename);
+            }
+            catch (IOException e)
+            {
+                Logging.errorPrint("Refusing to install entry that escapes the data directory: " + filename, e);
+                ShowMessageDelegate.showMessageDialog(
+                        LanguageBundle.getFormattedString("in_diWriteFail", filename),
+                        TITLE, MessageType.ERROR);
+                return false;
+            }
             if (new File(correctedFilename).exists())
             {
                 existingFiles.add(filename);
@@ -550,22 +563,52 @@ private static void copyInputStream(InputStream in, OutputStream out) throws IOE
      * Correct the file name to account for the selected data directory and
      * preference based folder locations such as output sheets.
      *
+     * <p>Resolved paths are validated against their base directory so a
+     * malicious archive entry containing {@code ..} segments cannot escape
+     * the install location ("Zip Slip"). Throws an {@link IOException} if
+     * the resolved entry would land outside its expected base.
+     *
      * @param destDir  the destination data directory
      * @param fileName the file name to be corrected.
      * @return the corrected file name.
+     * @throws IOException if the entry resolves outside its base directory
      */
-    private static String correctFileName(File destDir, String fileName)
+    static String correctFileName(File destDir, String fileName) throws IOException
     {
         if (fileName.toLowerCase().startsWith(DATA_FOLDER))
         {
-            fileName = destDir.getAbsolutePath() + fileName.substring(4);
-        } else if (fileName.toLowerCase().startsWith(OUTPUTSHEETS_FOLDER))
+            String suffix = fileName.substring(4);
+            return resolveWithinBase(destDir, suffix).getAbsolutePath();
+        }
+        else if (fileName.toLowerCase().startsWith(OUTPUTSHEETS_FOLDER))
         {
-            fileName = new File(ConfigurationSettings.getOutputSheetsDir()).getAbsolutePath() + fileName.substring(12);
+            String suffix = fileName.substring(12);
+            File outputSheetsDir = new File(ConfigurationSettings.getOutputSheetsDir());
+            return resolveWithinBase(outputSheetsDir, suffix).getAbsolutePath();
         }
         return fileName;
     }
 
+    /**
+     * Resolves {@code suffix} against {@code baseDir} and verifies the
+     * resulting path is contained within {@code baseDir}. Both sides are
+     * canonicalised (symlinks resolved, {@code .} and {@code ..} segments
+     * collapsed) before comparison so a hostile entry name like
+     * {@code ../../etc/passwd} cannot escape.
+     */
+    private static File resolveWithinBase(File baseDir, String suffix) throws IOException
+    {
+        File baseCanonical = baseDir.getCanonicalFile();
+        File resolved = new File(baseCanonical, suffix).getCanonicalFile();
+        Path basePath = baseCanonical.toPath();
+        if (!resolved.toPath().startsWith(basePath))
+        {
+            throw new IOException("Refusing to install entry that escapes "
+                    + baseCanonical + ": " + suffix);
+        }
+        return resolved;
+    }
+
     /**
      * Creates the directories needed by the installer, where they
      * do not already exist.
@@ -578,7 +621,19 @@ private static boolean createDirectories(Iterable<String> directories, File dest
     {
         for (String dirname : directories)
         {
-            String corrDirname = correctFileName(destDir, dirname);
+            String corrDirname;
+            try
+            {
+                corrDirname = correctFileName(destDir, dirname);
+            }
+            catch (IOException e)
+            {
+                Logging.errorPrint("Refusing to create directory from data set: " + dirname, e);
+                ShowMessageDelegate.showMessageDialog(
+                        LanguageBundle.getFormattedString("in_diDirNotCreated", dirname), TITLE,
+                        MessageType.ERROR);
+                return false;
+            }
             File dir = new File(corrDirname);
             if (!dir.exists())
             {

code/src/utest/pcgen/gui2/dialog/DataInstallerZipSlipTest.java

@@ -0,0 +1,89 @@
+/*
+ * Copyright 2026 Vest <Vest@users.noreply.github.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ */
+package pcgen.gui2.dialog;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.File;
+import java.io.IOException;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.nio.file.Path;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+/**
+ * Regression tests for the Zip Slip fix in {@link DataInstaller}. Reflection
+ * is used so the same test class compiles against both the pre-fix
+ * (private, no checked exception) and post-fix (package-private,
+ * throws IOException) signatures of {@code correctFileName}, which lets
+ * the test be exercised against either code state during the
+ * before/after verification dance.
+ */
+class DataInstallerZipSlipTest
+{
+	private static String invokeCorrectFileName(File destDir, String entry) throws IOException
+	{
+		try
+		{
+			Method m = DataInstaller.class.getDeclaredMethod(
+					"correctFileName", File.class, String.class);
+			m.setAccessible(true);
+			return (String) m.invoke(null, destDir, entry);
+		}
+		catch (InvocationTargetException e)
+		{
+			Throwable cause = e.getCause();
+			if (cause instanceof IOException ioe)
+			{
+				throw ioe;
+			}
+			throw new RuntimeException(cause);
+		}
+		catch (NoSuchMethodException | IllegalAccessException e)
+		{
+			throw new RuntimeException(e);
+		}
+	}
+
+	@Test
+	void benignEntryStaysUnderDestDir(@TempDir Path destDirPath) throws IOException
+	{
+		File destDir = destDirPath.toFile();
+
+		String resolved = invokeCorrectFileName(destDir, "data/sub/file.lst");
+
+		Path resolvedPath = new File(resolved).getCanonicalFile().toPath();
+		Path baseCanonical = destDir.getCanonicalFile().toPath();
+		assertTrue(resolvedPath.startsWith(baseCanonical),
+				"benign entry should resolve under destDir, got: " + resolvedPath);
+	}
+
+	@Test
+	void parentSegmentEscapeIsRejected(@TempDir Path destDirPath)
+	{
+		File destDir = destDirPath.toFile();
+
+		assertThrows(IOException.class,
+				() -> invokeCorrectFileName(destDir, "data/../../escape.txt"));
+	}
+
+	@Test
+	void buriedParentSegmentEscapeIsRejected(@TempDir Path destDirPath)
+	{
+		File destDir = destDirPath.toFile();
+
+		// Even when the '..' segments only escape after several levels of
+		// traversal, the canonical comparison should still catch them.
+		assertThrows(IOException.class,
+				() -> invokeCorrectFileName(destDir, "data/foo/bar/../../../../escape.txt"));
+	}
+}