From ac4a76a28b13a041a0018cacf698338d7f2d5ebb Mon Sep 17 00:00:00 2001 From: tmathern <60901087+tmathern@users.noreply.github.com> Date: Mon, 4 May 2026 16:11:25 -0700 Subject: [PATCH 1/2] fix: Invest --- include/c2pa.hpp | 17 ++++++++ src/c2pa_builder.cpp | 31 ++++++++++++++ src/c2pa_internal.hpp | 35 ++++++++++++++++ tests/builder.test.cpp | 94 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 177 insertions(+) diff --git a/include/c2pa.hpp b/include/c2pa.hpp index 0a0d5a5..4beb8f7 100644 --- a/include/c2pa.hpp +++ b/include/c2pa.hpp @@ -1199,6 +1199,9 @@ namespace c2pa /// @param dest The output stream to write the signed data to. /// @param signer The Signer object to use for signing. /// @return A vector containing the signed manifest bytes. + /// @pre source and dest must be distinct streams; they must not share an + /// underlying std::streambuf. In-place signing is not supported and + /// throws C2paException without modifying the source data. /// @throws C2paException for errors encountered by the C2PA library. /// @deprecated Use sign(const string&, std::istream&, std::iostream&, Signer&) instead. std::vector sign(const std::string &format, std::istream &source, std::ostream &dest, Signer &signer); @@ -1209,6 +1212,9 @@ namespace c2pa /// @param dest The I/O stream to write the signed data to. /// @param signer The Signer object to use for signing. /// @return A vector containing the signed manifest bytes. + /// @pre source and dest must be distinct streams; they must not share an + /// underlying std::streambuf. In-place signing is not supported and + /// throws C2paException without modifying the source data. /// @throws C2paException for errors encountered by the C2PA library. std::vector sign(const std::string &format, std::istream &source, std::iostream &dest, Signer &signer); @@ -1217,6 +1223,10 @@ namespace c2pa /// @param dest_path The path to write the signed file to. /// @param signer The signer object to use for signing. /// @return A vector containing the signed manifest bytes. + /// @pre source_path and dest_path must refer to different files. In-place + /// signing is not supported; passing the same path (or two paths that + /// resolve to the same filesystem entity) throws C2paException without + /// modifying the source. /// @throws C2paException for errors encountered by the C2PA library. /// @note Prefer using the streaming APIs if possible. std::vector sign(const std::filesystem::path &source_path, const std::filesystem::path &dest_path, Signer &signer); @@ -1230,6 +1240,9 @@ namespace c2pa /// @param source The input stream to sign. /// @param dest The I/O stream to write the signed data to. /// @return A vector containing the signed manifest bytes. + /// @pre source and dest must be distinct streams; they must not share an + /// underlying std::streambuf. In-place signing is not supported and + /// throws C2paException without modifying the source data. /// @throws C2paException if the context has no signer or on other errors. std::vector sign(const std::string &format, std::istream &source, std::iostream &dest); @@ -1241,6 +1254,10 @@ namespace c2pa /// @param source_path The path to the file to sign. /// @param dest_path The path to write the signed file to. /// @return A vector containing the signed manifest bytes. + /// @pre source_path and dest_path must refer to different files. In-place + /// signing is not supported; passing the same path (or two paths that + /// resolve to the same filesystem entity) throws C2paException without + /// modifying the source. /// @throws C2paException if the context has no signer or on other errors. std::vector sign(const std::filesystem::path &source_path, const std::filesystem::path &dest_path); diff --git a/src/c2pa_builder.cpp b/src/c2pa_builder.cpp index 686dd8a..57d78dc 100644 --- a/src/c2pa_builder.cpp +++ b/src/c2pa_builder.cpp @@ -221,6 +221,11 @@ namespace c2pa std::vector Builder::sign(const std::string &format, std::istream &source, std::ostream &dest, Signer &signer) { + if (detail::streams_alias(source, dest)) { + throw C2paException( + "Source and destination streams share the same underlying buffer; " + "in-place signing is not supported and would corrupt the source data."); + } // Caller's source/dest streams must outlive this call // Stream wrappers are stack locals that wrap the caller's streams CppIStream c_source(source); @@ -234,6 +239,11 @@ namespace c2pa std::vector Builder::sign(const std::string &format, std::istream &source, std::iostream &dest, Signer &signer) { + if (detail::streams_alias(source, dest)) { + throw C2paException( + "Source and destination streams share the same underlying buffer; " + "in-place signing is not supported and would corrupt the source data."); + } // Caller's source/dest streams must outlive this call // Stream wrappers are stack locals that wrap the caller's streams CppIStream c_source(source); @@ -250,9 +260,17 @@ namespace c2pa /// @param dest_path The path to write the signed file to. /// @param signer A signer object to use when signing. /// @return A vector containing the signed manifest bytes. + /// @pre source_path and dest_path must refer to different files. In-place signing + /// is not supported; passing the same path (or two paths that resolve to the + /// same filesystem entity) throws C2paException without modifying the source. /// @throws C2pa::C2paException for errors encountered by the C2PA library. std::vector Builder::sign(const std::filesystem::path &source_path, const std::filesystem::path &dest_path, Signer &signer) { + if (detail::paths_alias(source_path, dest_path)) { + throw C2paException( + "Source and destination must differ; " + "in-place signing is not supported and would corrupt the source file."); + } auto source = detail::open_file_binary(source_path); // Ensure the destination directory exists auto dest_dir = dest_path.parent_path(); @@ -276,6 +294,11 @@ namespace c2pa std::vector Builder::sign(const std::string &format, std::istream &source, std::iostream &dest) { + if (detail::streams_alias(source, dest)) { + throw C2paException( + "Source and destination streams share the same underlying buffer; " + "in-place signing is not supported and would corrupt the source data."); + } CppIStream c_source(source); CppIOStream c_dest(dest); const unsigned char *c2pa_manifest_bytes = nullptr; @@ -284,8 +307,16 @@ namespace c2pa return detail::to_byte_vector(c2pa_manifest_bytes, result); } + /// @pre source_path and dest_path must refer to different files. In-place signing + /// is not supported; passing the same path (or two paths that resolve to the + /// same filesystem entity) throws C2paException without modifying the source. std::vector Builder::sign(const std::filesystem::path &source_path, const std::filesystem::path &dest_path) { + if (detail::paths_alias(source_path, dest_path)) { + throw C2paException( + "Source and destination must differ; " + "in-place signing is not supported and would corrupt the source file."); + } auto source = detail::open_file_binary(source_path); auto dest_dir = dest_path.parent_path(); if (!std::filesystem::exists(dest_dir)) diff --git a/src/c2pa_internal.hpp b/src/c2pa_internal.hpp index e1fe8ab..fbd9589 100644 --- a/src/c2pa_internal.hpp +++ b/src/c2pa_internal.hpp @@ -21,7 +21,11 @@ #include #include #include +#include +#include +#include #include +#include #include #include @@ -217,6 +221,37 @@ inline std::string extract_file_extension(const std::filesystem::path &path) noe return ext.empty() ? "" : ext.substr(1); } +/// @brief Test whether two paths refer to the same filesystem entity. +/// Returns false on any filesystem error rather than throwing. +inline bool paths_alias(const std::filesystem::path &a, + const std::filesystem::path &b) noexcept { + namespace fs = std::filesystem; + std::error_code ec; + const bool a_exists = fs::exists(a, ec); + if (ec) { return false; } + const bool b_exists = fs::exists(b, ec); + if (ec) { return false; } + if (a_exists && b_exists) { + const bool eq = fs::equivalent(a, b, ec); + return !ec && eq; + } + auto ca = fs::weakly_canonical(a, ec); + if (ec) { return false; } + auto cb = fs::weakly_canonical(b, ec); + if (ec) { return false; } + return ca == cb; +} + +/// @brief Test whether two C++ streams share the same underlying buffer. +/// @details Compares std::streambuf pointers via rdbuf(). +/// Returns false if either rdbuf is null, since a null source rdbuf is +/// independently broken and will fail at the first read attempt. +inline bool streams_alias(const std::ios &a, const std::ios &b) noexcept { + std::streambuf *ba = a.rdbuf(); + std::streambuf *bb = b.rdbuf(); + return ba != nullptr && ba == bb; +} + /// @brief Convert C string result to C++ string with cleanup /// @param c_result Raw C string from C API /// @return C++ string (throws if null) diff --git a/tests/builder.test.cpp b/tests/builder.test.cpp index 11d7cdd..0f29407 100644 --- a/tests/builder.test.cpp +++ b/tests/builder.test.cpp @@ -855,6 +855,100 @@ TEST_F(BuilderTest, SignImageFileOnly) ASSERT_TRUE(std::filesystem::exists(output_path)); }; +TEST_F(BuilderTest, SignFile_SameSourceAndDest_ThrowsWithoutTouchingSource) +{ + fs::path current_dir = fs::path(__FILE__).parent_path(); + fs::path manifest_path = current_dir / "../tests/fixtures/training.json"; + fs::path certs_path = current_dir / "../tests/fixtures/es256_certs.pem"; + fs::path src_asset = current_dir / "../tests/fixtures/A.jpg"; + + fs::path inplace_path = get_temp_path("inplace_same_path.jpg"); + fs::copy_file(src_asset, inplace_path, fs::copy_options::overwrite_existing); + auto orig_size = fs::file_size(inplace_path); + ASSERT_GT(orig_size, 0u); + + auto manifest = c2pa_test::read_text_file(manifest_path); + auto certs = c2pa_test::read_text_file(certs_path); + auto p_key = c2pa_test::read_text_file(current_dir / "../tests/fixtures/es256_private.key"); + c2pa::Signer signer("Es256", certs, p_key, "http://timestamp.digicert.com"); + c2pa::Builder builder(manifest); + + EXPECT_THROW(builder.sign(inplace_path, inplace_path, signer), c2pa::C2paException); + EXPECT_EQ(fs::file_size(inplace_path), orig_size) + << "Source file must be untouched when sign rejects aliased paths."; +} + +TEST_F(BuilderTest, SignFile_SameSourceAndDest_NoSigner_ThrowsWithoutTouchingSource) +{ + fs::path current_dir = fs::path(__FILE__).parent_path(); + fs::path src_asset = current_dir / "../tests/fixtures/A.jpg"; + + fs::path inplace_path = get_temp_path("inplace_same_path_nosigner.jpg"); + fs::copy_file(src_asset, inplace_path, fs::copy_options::overwrite_existing); + auto orig_size = fs::file_size(inplace_path); + ASSERT_GT(orig_size, 0u); + + fs::path manifest_path = current_dir / "../tests/fixtures/training.json"; + auto manifest = c2pa_test::read_text_file(manifest_path); + c2pa::Builder builder(manifest); + + EXPECT_THROW(builder.sign(inplace_path, inplace_path), c2pa::C2paException); + EXPECT_EQ(fs::file_size(inplace_path), orig_size); +} + +TEST_F(BuilderTest, SignStream_SameIOStream_Throws) +{ + fs::path current_dir = fs::path(__FILE__).parent_path(); + fs::path manifest_path = current_dir / "../tests/fixtures/training.json"; + fs::path certs_path = current_dir / "../tests/fixtures/es256_certs.pem"; + fs::path src_asset = current_dir / "../tests/fixtures/A.jpg"; + + std::ifstream in(src_asset, std::ios::binary); + ASSERT_TRUE(in.is_open()); + std::stringstream both(std::ios::in | std::ios::out | std::ios::binary); + both << in.rdbuf(); + auto orig_bytes = both.str(); + ASSERT_FALSE(orig_bytes.empty()); + + auto manifest = c2pa_test::read_text_file(manifest_path); + auto certs = c2pa_test::read_text_file(certs_path); + auto p_key = c2pa_test::read_text_file(current_dir / "../tests/fixtures/es256_private.key"); + c2pa::Signer signer("Es256", certs, p_key, "http://timestamp.digicert.com"); + c2pa::Builder builder(manifest); + + std::iostream &both_io = both; + EXPECT_THROW(builder.sign("image/jpeg", both_io, both_io, signer), c2pa::C2paException); + EXPECT_EQ(both.str(), orig_bytes) + << "Stream buffer must be untouched when sign rejects aliased streams."; +} + +TEST_F(BuilderTest, SignStream_DistinctStreamsSharingBuffer_Throws) +{ + // Two distinct stream wrappers around one streambuf should be rejected. + fs::path current_dir = fs::path(__FILE__).parent_path(); + fs::path manifest_path = current_dir / "../tests/fixtures/training.json"; + fs::path certs_path = current_dir / "../tests/fixtures/es256_certs.pem"; + fs::path src_asset = current_dir / "../tests/fixtures/A.jpg"; + + std::ifstream in(src_asset, std::ios::binary); + ASSERT_TRUE(in.is_open()); + std::stringstream backing(std::ios::in | std::ios::out | std::ios::binary); + backing << in.rdbuf(); + auto orig_bytes = backing.str(); + + std::istream src_view(backing.rdbuf()); + std::iostream dest_view(backing.rdbuf()); + + auto manifest = c2pa_test::read_text_file(manifest_path); + auto certs = c2pa_test::read_text_file(certs_path); + auto p_key = c2pa_test::read_text_file(current_dir / "../tests/fixtures/es256_private.key"); + c2pa::Signer signer("Es256", certs, p_key, "http://timestamp.digicert.com"); + c2pa::Builder builder(manifest); + + EXPECT_THROW(builder.sign("image/jpeg", src_view, dest_view, signer), c2pa::C2paException); + EXPECT_EQ(backing.str(), orig_bytes); +} + TEST_F(BuilderTest, SignImageFileNoThumbnailAutoGenThreadLocalSettings) { // Run in separate thread for complete test isolation (thread-local settings won't leak) From 1fb2d2e3684de2edddbd264d888695b0144f13c3 Mon Sep 17 00:00:00 2001 From: tmathern <60901087+tmathern@users.noreply.github.com> Date: Mon, 4 May 2026 16:13:11 -0700 Subject: [PATCH 2/2] fix: Invest --- src/c2pa_internal.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/c2pa_internal.hpp b/src/c2pa_internal.hpp index fbd9589..292f4e6 100644 --- a/src/c2pa_internal.hpp +++ b/src/c2pa_internal.hpp @@ -242,7 +242,7 @@ inline bool paths_alias(const std::filesystem::path &a, return ca == cb; } -/// @brief Test whether two C++ streams share the same underlying buffer. +/// @brief Test whether two streams share the same underlying buffer. /// @details Compares std::streambuf pointers via rdbuf(). /// Returns false if either rdbuf is null, since a null source rdbuf is /// independently broken and will fail at the first read attempt.