Skip to content

Fix java/local-temp-file-or-directory-information-disclosure CodeQL alerts by using NIO temp file APIs#763

Open
vharseko wants to merge 2 commits into
OpenIdentityPlatform:masterfrom
vharseko:fix-codeql-java-temp-file
Open

Fix java/local-temp-file-or-directory-information-disclosure CodeQL alerts by using NIO temp file APIs#763
vharseko wants to merge 2 commits into
OpenIdentityPlatform:masterfrom
vharseko:fix-codeql-java-temp-file

Conversation

@vharseko

Copy link
Copy Markdown
Member

Fixes the twelve open java/local-temp-file-or-directory-information-disclosure code-scanning alerts (#116#127):

  • Replaces File.createTempFile(...) (creates world-readable files, 0644) with Files.createTempFile(...).toFile(), which creates files readable/writable only by the owner (0600) on POSIX systems: ControlPanelLauncher, NewBaseDNPanel, TempLogFile, InstallerHelper, OnDiskMergeImporter, SchemaFilesWriter, LDAPAuthenticationHandler, ReplicationCliMain, StatusCli, SetupUtils.
  • EmbeddedOpenDJ: the server root is now created with Files.createTempDirectory("opendj") (0700) instead of the fixed shared {java.io.tmpdir}/opendj directory created with mkdir(). This also removes the collision between two embedded instances (or two users on the same machine) using the same fixed path; the path was never referenced externally.
  • ConfigGuideGeneration (doc-generation tool): the default output directory is now created with Files.createTempDirectory; behavior is unchanged when the GenerationDir property is set.

@vharseko vharseko added the security Security fixes / CodeQL code-scanning alerts label Jul 24, 2026
@vharseko
vharseko requested a review from maximthomas July 24, 2026 16:30
@vharseko vharseko added the java Pull requests that update java code label Jul 24, 2026
Setup from an archive requires the server root directory to be named
after the root directory contained in the archive ('opendj'), but
Files.createTempDirectory appends a random suffix to the name. Create
the random per-instance directory as a parent and the 'opendj' server
root inside it, keeping the temp directory private to the current user.
generationDir = Files.createTempDirectory(CONFIG_GUIDE_DIR).toString();
} else {
// Create new dir if necessary
new File(generationDir).mkdir();

@maximthomas maximthomas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The switch to Files.createTempFile / Files.createTempDirectory is the right fix and I verified it works (rw------- vs rw-rw-r-- on JDK 17, and the downstream writers truncate rather than recreate, so the mode survives). Two things need addressing before merge.

Embedded server root is never cleaned up (high)

opendj-embedded/src/main/java/org/openidentityplatform/opendj/embedded/EmbeddedOpenDJ.java

The old code wiped the fixed {tmpdir}/opendj at startup, so at most one stale root existed. The new per-instance directory is never deleted: close() only stops the server, there is no deleteOnExit, and EmbeddedDirectoryServer does no cleanup either.

Each root holds the copied opendj.zip (81 MB) plus its extracted contents (97.5 MB, 325 files) plus the JE database — ~180 MB leaked per new EmbeddedOpenDJ(...). A test suite or a service that restarts a few times will fill /tmp.

Keep a reference and delete it on close:

private final File instanceDirectory;
...
this.instanceDirectory = Files.createTempDirectory("opendj").toFile();
File rootDirectory = new File(instanceDirectory, "opendj");
rootDirectory.mkdir();

@Override
public void close() {
    if (server.isRunning()) { ... }
    FileUtils.deleteQuietly(instanceDirectory);   // idempotent: close() is also a shutdown hook
}

Installed schema files silently drop to 0600 (medium)

opendj-server-legacy/src/main/java/org/opends/server/schema/SchemaFilesWriter.java:1071

Files.copy does propagate the source's permission bits on Unix — COPY_ATTRIBUTES only adds timestamps/ownership. Verified on JDK 17: copying a 0600 source over a rw-r--r-- target leaves the target rw-------.

So the now-0600 temp file at line 729 propagates into the live install:

Files.copy(tempFile.toPath(), installedFile.toPath(), StandardCopyOption.REPLACE_EXISTING);

config/schema/*.ldif changes from 0644/0664 to 0600 after any LDAP schema modification, while the schema files shipped by setup keep their original mode — so an instance ends up with mixed permissions. This is persistent install state, outside the PR's stated temp-file scope, and it is not mentioned in the description.

Probably harmless since the server process owns the files, but it should be a deliberate choice. Either preserve the previous mode:

boolean posix = installedFile.toPath().getFileSystem().supportedFileAttributeViews().contains("posix");
Set<PosixFilePermission> previous = posix && installedFile.exists()
    ? Files.getPosixFilePermissions(installedFile.toPath()) : null;
Files.copy(tempFile.toPath(), installedFile.toPath(), StandardCopyOption.REPLACE_EXISTING);
if (previous != null) {
    Files.setPosixFilePermissions(installedFile.toPath(), previous);
}

or keep 0600 intentionally and say so in the PR description.

Nits

  • Unused imports: import java.io.File; is now dead in opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/ControlPanelLauncher.java:24 and opendj-server-legacy/src/main/java/org/opends/server/tools/status/StatusCli.java:32 — the replaced File.createTempFile call was the only remaining use in both.
  • Embedded root path no longer discoverable: callers could previously compute {java.io.tmpdir}/opendj; the path is now random and EmbeddedOpenDJ neither logs it nor exposes a getter. Add logger.info("OpenDJ server root: {}", rootDirectory) and consider a getServerRootDirectory() accessor; worth a release-note line for the module.
  • Stale javadoc: opendj-server-legacy/src/main/java/org/opends/server/admin/doc/ConfigGuideGeneration.java:101 still documents the default as /var/tmp/[CONFIG_GUIDE_DIR]. (Good call keeping the println at line 127 — the random dir stays discoverable.)
  • POSIX-only: on Windows Files.createTempFile without a FileAttribute uses default ACLs, identical to before. The alerts are silenced everywhere but the hardening is real only on POSIX — worth stating in the description.
  • Copyright wording: PR uses Portions Copyrighted 2026 3A Systems, LLC.; the repo convention is Portions Copyright ... 3A Systems (243 files vs 13).
  • Import order: opendj-server-legacy/src/main/java/org/opends/server/util/SetupUtils.java puts java.nio.file.Files before java.net.InetSocketAddress.
  • Tests: none added. opendj-embedded/src/test/java/org/openidentityplatform/opendj/embedded/EmbeddedOpenDJTest.java passes with the leak in place. Two cheap guards: assert the root directory is gone after close() (once fixed), and assert Files.getPosixFilePermissions(...) is rw------- for one representative helper such as SetupUtils.createTemplateFile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

java Pull requests that update java code security Security fixes / CodeQL code-scanning alerts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants