uber / h3-java

Java bindings for H3, a hierarchical hexagonal geospatial indexing system
https://uber.github.io/h3/
Apache License 2.0
273 stars 53 forks source link

Use safer API for creating temporary files to publicly writable directories #140

Closed rng70-or closed 2 months ago

rng70-or commented 3 months ago

In file: H3CoreLoader.java, there is a method loadNatives that creates a temporary file using a hard-coded path to a public writable directory. This exposes the temporarily created file to race condition attacks. An attacker can access, modify or even delete a file. so a temporary file should be created using a safe API in a secured directory instead.

Possible Fix

a possible fix is suggested below

if(SystemUtils.IS_OS_UNIX) {
       FileAttribute<Set<PosixFilePermission>> attr = PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rwx------"));
       Files.createTempFile("prefix", "suffix", attr);
   }
   else {
       File f = Files.createTempFile("prefix", "suffix").toFile();
       f.setReadable(true, true);
       f.setWritable(true, true);
       f.setExecutable(true, true);
 }

or any kind of safer api is recommended.

Sponsorship and Support

This work is done by the security researchers from OpenRefactory and is supported by the Open Source Security Foundation (OpenSSF): Project Alpha-Omega. Alpha-Omega is a project partnering with open source software project maintainers to systematically find new, as-yet-undiscovered vulnerabilities in open source code - and get them fixed – to improve global software supply chain security.

The bug is found by running the Intelligent Code Repair (iCR) tool by OpenRefactory and then manually triaging the results.

isaacbrodsky commented 3 months ago

Thanks for reporting this. I opened #141 to apply the change.

From what I can see of the Java implementation, simply using Files.createTempFile instead of File.createTempFile may be enough to apply this without any other change, but something along the lines of your proposal may be necessary to be sufficiently paranoid. :)