From fc90e00a210e83283ca7c5dfc353db912f07f17d Mon Sep 17 00:00:00 2001 From: waynercheung Date: Thu, 16 Jul 2026 19:31:09 +0800 Subject: [PATCH] fix(zksnark): harden sapling param extraction Use secure temporary files and restrictive POSIX permissions when extracting packaged Sapling parameters. Handle missing resources explicitly and remove partial files when extraction fails. Add regression tests for the extraction lifecycle. --- .../org/tron/core/zen/ZksnarkInitService.java | 83 ++++++++++-- .../tron/core/zen/ZksnarkInitServiceTest.java | 127 ++++++++++++++++++ .../test/resources/params/test-zksnark.params | 1 + 3 files changed, 198 insertions(+), 13 deletions(-) create mode 100644 framework/src/test/java/org/tron/core/zen/ZksnarkInitServiceTest.java create mode 100644 framework/src/test/resources/params/test-zksnark.params diff --git a/framework/src/main/java/org/tron/core/zen/ZksnarkInitService.java b/framework/src/main/java/org/tron/core/zen/ZksnarkInitService.java index dfc4b428836..c1953c900f0 100644 --- a/framework/src/main/java/org/tron/core/zen/ZksnarkInitService.java +++ b/framework/src/main/java/org/tron/core/zen/ZksnarkInitService.java @@ -1,12 +1,22 @@ package org.tron.core.zen; -import java.io.File; +import com.google.common.annotations.VisibleForTesting; import java.io.IOException; import java.io.InputStream; +import java.io.OutputStream; +import java.nio.file.Files; +import java.nio.file.LinkOption; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFilePermissions; +import java.util.Collections; +import java.util.EnumSet; +import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import javax.annotation.PostConstruct; import lombok.extern.slf4j.Slf4j; -import org.apache.commons.io.FileUtils; +import org.apache.commons.io.IOUtils; import org.springframework.stereotype.Component; import org.tron.common.zksnark.JLibrustzcash; import org.tron.common.zksnark.LibrustzcashParam; @@ -18,6 +28,9 @@ public class ZksnarkInitService { private static final AtomicBoolean initialized = new AtomicBoolean(false); + private static final Set OWNER_ONLY = + Collections.unmodifiableSet(EnumSet.of( + PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE)); @PostConstruct private void init() { @@ -56,19 +69,63 @@ public static void librustzcashInitZksnarkParams() { } } - private static String getParamsFile(String fileName) { - InputStream in = Thread.currentThread().getContextClassLoader() - .getResourceAsStream("params" + File.separator + fileName); - File fileOut = new File(System.getProperty("java.io.tmpdir") - + File.separator + fileName + "." + System.currentTimeMillis()); + @VisibleForTesting + static String getParamsFile(String fileName) { + InputStream resource = ZksnarkInitService.class.getResourceAsStream("/params/" + fileName); + if (resource == null) { + throw new TronError("Missing zk param resource: " + fileName, + TronError.ErrCode.ZCASH_INIT); + } + + Path fileOut = null; + try (InputStream in = resource) { + fileOut = copyToSecureTempFile(in, fileName); + fileOut.toFile().deleteOnExit(); + return fileOut.toAbsolutePath().toString(); + } catch (IOException | RuntimeException e) { + deleteTempFile(fileOut, e); + throw new TronError("Failed to release zk param resource: " + fileName, e, + TronError.ErrCode.ZCASH_INIT); + } + } + + @VisibleForTesting + static Path copyToSecureTempFile(InputStream in, String fileName) throws IOException { + int dotIndex = fileName.lastIndexOf('.'); + String prefix = (dotIndex > 0 ? fileName.substring(0, dotIndex) : fileName) + "-"; + String suffix = dotIndex > 0 ? fileName.substring(dotIndex) : ".params"; + Path fileOut = createTempFile(prefix, suffix); + + try (OutputStream out = Files.newOutputStream(fileOut, + StandardOpenOption.WRITE, + StandardOpenOption.TRUNCATE_EXISTING, + LinkOption.NOFOLLOW_LINKS)) { + IOUtils.copyLarge(in, out); + return fileOut; + } catch (IOException | RuntimeException e) { + deleteTempFile(fileOut, e); + throw e; + } + } + + private static Path createTempFile(String prefix, String suffix) throws IOException { try { - FileUtils.copyToFile(in, fileOut); - } catch (IOException e) { - logger.error(e.getMessage(), e); + return Files.createTempFile(prefix, suffix, + PosixFilePermissions.asFileAttribute(OWNER_ONLY)); + } catch (UnsupportedOperationException e) { + // Non-POSIX providers still create the unpredictable name atomically. + return Files.createTempFile(prefix, suffix); } - if (fileOut.exists()) { - fileOut.deleteOnExit(); + } + + private static void deleteTempFile(Path file, Throwable failure) { + if (file == null) { + return; + } + try { + Files.deleteIfExists(file); + } catch (IOException | RuntimeException cleanupError) { + failure.addSuppressed(cleanupError); } - return fileOut.getAbsolutePath(); } } diff --git a/framework/src/test/java/org/tron/core/zen/ZksnarkInitServiceTest.java b/framework/src/test/java/org/tron/core/zen/ZksnarkInitServiceTest.java new file mode 100644 index 00000000000..4942d5ebb54 --- /dev/null +++ b/framework/src/test/java/org/tron/core/zen/ZksnarkInitServiceTest.java @@ -0,0 +1,127 @@ +package org.tron.core.zen; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.LinkOption; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.attribute.PosixFileAttributeView; +import java.nio.file.attribute.PosixFilePermission; +import java.util.ArrayList; +import java.util.EnumSet; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import org.apache.commons.io.IOUtils; +import org.junit.After; +import org.junit.Test; +import org.tron.core.exception.TronError; + +public class ZksnarkInitServiceTest { + + private static final String TEST_PARAM_FILE = "test-zksnark.params"; + private static final Set OWNER_ONLY = + EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE); + + private final List tempFiles = new ArrayList<>(); + + @After + public void tearDown() throws IOException { + for (Path tempFile : tempFiles) { + Files.deleteIfExists(tempFile); + } + } + + @Test + public void testGetParamsFileCreatesSecureTempFile() throws IOException { + Path file = track(ZksnarkInitService.getParamsFile(TEST_PARAM_FILE)); + + assertTrue(Files.isRegularFile(file, LinkOption.NOFOLLOW_LINKS)); + assertFalse(Files.isSymbolicLink(file)); + if (Files.getFileStore(file).supportsFileAttributeView(PosixFileAttributeView.class)) { + assertEquals(OWNER_ONLY, Files.getPosixFilePermissions(file)); + } + assertTrue(file.getFileName().toString().startsWith("test-zksnark-")); + assertTrue(file.getFileName().toString().endsWith(".params")); + + try (InputStream expected = ZksnarkInitService.class + .getResourceAsStream("/params/" + TEST_PARAM_FILE); + InputStream actual = Files.newInputStream(file)) { + assertTrue(IOUtils.contentEquals(expected, actual)); + } + } + + @Test + public void testGetParamsFileUsesUniqueNames() { + Path first = track(ZksnarkInitService.getParamsFile(TEST_PARAM_FILE)); + Path second = track(ZksnarkInitService.getParamsFile(TEST_PARAM_FILE)); + + assertNotEquals(first, second); + } + + @Test + public void testGetParamsFileMissingResourceThrows() { + TronError exception = assertThrows(TronError.class, + () -> ZksnarkInitService.getParamsFile("does-not-exist.params")); + + assertEquals(TronError.ErrCode.ZCASH_INIT, exception.getErrCode()); + } + + @Test + public void testCopyFailureDeletesPartialFile() throws IOException { + String fileName = "failing-zksnark-" + System.nanoTime() + ".params"; + Set filesBefore = findTempFiles(fileName); + + InputStream failingInput = new InputStream() { + private boolean firstRead = true; + + @Override + public int read() throws IOException { + if (firstRead) { + firstRead = false; + return 1; + } + throw new IOException("expected copy failure"); + } + + @Override + public int read(byte[] buffer, int offset, int length) throws IOException { + if (firstRead) { + firstRead = false; + buffer[offset] = 1; + return 1; + } + throw new IOException("expected copy failure"); + } + }; + + assertThrows(IOException.class, + () -> ZksnarkInitService.copyToSecureTempFile(failingInput, fileName)); + assertEquals(filesBefore, findTempFiles(fileName)); + } + + private Path track(String fileName) { + Path file = Paths.get(fileName); + tempFiles.add(file); + return file; + } + + private static Set findTempFiles(String fileName) throws IOException { + int dotIndex = fileName.lastIndexOf('.'); + String prefix = (dotIndex > 0 ? fileName.substring(0, dotIndex) : fileName) + "-"; + Path tempDirectory = Paths.get(System.getProperty("java.io.tmpdir")); + try (Stream files = Files.list(tempDirectory)) { + return files + .filter(path -> path.getFileName().toString().startsWith(prefix)) + .collect(Collectors.toSet()); + } + } +} diff --git a/framework/src/test/resources/params/test-zksnark.params b/framework/src/test/resources/params/test-zksnark.params new file mode 100644 index 00000000000..5389a4b9c93 --- /dev/null +++ b/framework/src/test/resources/params/test-zksnark.params @@ -0,0 +1 @@ +java-tron secure temporary zk parameter test resource