[Pcgen-github] [PCGen/pcgen] c4cbc4: fix: address CodeQL findings (zip-slip + implicit ...

[Vest <no...@gi...>]

  Branch: refs/heads/master
  Home:   https://github.com/PCGen/pcgen
  Commit: c4cbc43ba1f8a283791d4426c79d54bbdee68998
      https://github.com/PCGen/pcgen/commit/c4cbc43ba1f8a283791d4426c79d54bbdee68998
  Author: Vest <Ve...@us...>
  Date:   2026-06-03 (Wed, 03 Jun 2026)

  Changed paths:
    M code/src/java/pcgen/core/analysis/SkillModifier.java
    M code/src/java/pcgen/gui2/dialog/DataInstaller.java
    A code/src/utest/pcgen/gui2/dialog/DataInstallerZipSlipTest.java

  Log Message:
  -----------
  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.



To unsubscribe from these emails, change your notification settings at https://github.com/PCGen/pcgen/settings/notifications