From 8f7a928ca1883ff2d1ee2ae33309d09b902df77c Mon Sep 17 00:00:00 2001 From: Peter Colberg Date: Fri, 28 Oct 2022 13:13:44 -0400 Subject: [PATCH 1/3] pkg_editor_test: resolve stack-use-after-scope in create_file() The buffer passed to acl_pkg_add_data_section() is read later in acl_pkg_close_file() which invokes elf_update() to flush dirty information associated with the ELF file descriptor to disk, at which point the buffer has already gone out of scope. --- lib/pkg_editor/test/pkg_editor_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pkg_editor/test/pkg_editor_test.cpp b/lib/pkg_editor/test/pkg_editor_test.cpp index da9d6317..37699c76 100644 --- a/lib/pkg_editor/test/pkg_editor_test.cpp +++ b/lib/pkg_editor/test/pkg_editor_test.cpp @@ -102,7 +102,7 @@ struct acl_pkg_file *create_file() { pkg = acl_pkg_open_file(SAMPLE_FILE, ACL_PKG_CREATE | ACL_PKG_READ_WRITE); CHECK(pkg); - char name[] = "13.0.0"; + const static char name[] = "13.0.0"; CHECK(acl_pkg_add_data_section(pkg, ACL_PKG_SECTION_ACL_VERSION, name, sizeof(name))); return pkg; From 98c776b391cf82be2c79c5e6ad7ef29306338990 Mon Sep 17 00:00:00 2001 From: Peter Colberg Date: Fri, 28 Oct 2022 14:36:38 -0400 Subject: [PATCH 2/3] pkg_editor_test: resolve stack-buffer-overflow calling acl_pkg_read_section() acl_pkg_read_section() appends a null byte to the section data, which had not been accounted for in the target buffer sizes. --- lib/pkg_editor/test/pkg_editor_test.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/pkg_editor/test/pkg_editor_test.cpp b/lib/pkg_editor/test/pkg_editor_test.cpp index 37699c76..c8ea0620 100644 --- a/lib/pkg_editor/test/pkg_editor_test.cpp +++ b/lib/pkg_editor/test/pkg_editor_test.cpp @@ -170,12 +170,12 @@ TEST(sample_file, write_ops_on_writable) { acl_pkg_add_data_section(pkg, ACL_PKG_SECTION_HASH, &data, sizeof(data))); // Check that we get the right data back. size_t data_size; - size_t data_result = 99; CHECK(acl_pkg_section_exists(pkg, ACL_PKG_SECTION_HASH, &data_size)); CHECK_EQUAL(sizeof(data), data_size); - CHECK(acl_pkg_read_section(pkg, ACL_PKG_SECTION_HASH, (char *)&data_result, - data_size + 1)); - CHECK_EQUAL(data, data_result); + char data_result[sizeof(data) + 1] = {0}; + CHECK(acl_pkg_read_section(pkg, ACL_PKG_SECTION_HASH, data_result, + sizeof(data_result))); + CHECK_EQUAL(0, memcmp(&data, data_result, sizeof(data))); char *data_result_ptr = 0; CHECK(acl_pkg_read_section_transient(pkg, ACL_PKG_SECTION_HASH, &data_result_ptr)); @@ -187,10 +187,10 @@ TEST(sample_file, write_ops_on_writable) { // Check that we get the right data back. CHECK(acl_pkg_section_exists(pkg, ACL_PKG_SECTION_HASH, &data_size)); CHECK_EQUAL(sizeof(data), data_size); - data_result = 99; - CHECK(acl_pkg_read_section(pkg, ACL_PKG_SECTION_HASH, (char *)&data_result, - data_size + 1)); - CHECK_EQUAL(data2, data_result); + char data2_result[sizeof(data) + 1] = {0}; + CHECK(acl_pkg_read_section(pkg, ACL_PKG_SECTION_HASH, data2_result, + sizeof(data2_result))); + CHECK_EQUAL(0, memcmp(&data2, data2_result, sizeof(data2))); data_result_ptr = 0; CHECK(acl_pkg_read_section_transient(pkg, ACL_PKG_SECTION_HASH, &data_result_ptr)); @@ -230,9 +230,9 @@ TEST(sample_file, read_readonly) { CHECK(pkg); CHECK(acl_pkg_section_exists(pkg, ACL_PKG_SECTION_HASH, &data_size)); CHECK_EQUAL(sizeof(hw), data_size); - char data_result[sizeof(hw)]; + char data_result[sizeof(hw) + 1] = {0}; CHECK(acl_pkg_read_section(pkg, ACL_PKG_SECTION_HASH, data_result, - data_size + 1)); + sizeof(data_result))); CHECK_EQUAL(0, strcmp(data_result, hw)); char *data_result_ptr = 0; CHECK(acl_pkg_read_section_transient(pkg, ACL_PKG_SECTION_HASH, From aaefd81bf1a3ae45b7c6d710c814643e87a7b0f8 Mon Sep 17 00:00:00 2001 From: Peter Colberg Date: Fri, 28 Oct 2022 17:15:25 -0400 Subject: [PATCH 3/3] pkg_editor_test: fix memory leaks due to missing acl_pkg_close_file() --- lib/pkg_editor/test/pkg_editor_test.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/pkg_editor/test/pkg_editor_test.cpp b/lib/pkg_editor/test/pkg_editor_test.cpp index c8ea0620..33650101 100644 --- a/lib/pkg_editor/test/pkg_editor_test.cpp +++ b/lib/pkg_editor/test/pkg_editor_test.cpp @@ -153,6 +153,7 @@ TEST(sample_file, write_ops_on_readonly) { sizeof(data))); CHECK_EQUAL(0, acl_pkg_update_section_from_file(pkg, ACL_PKG_SECTION_HASH, SAMPLE_FILE)); + close_file(pkg); printf("end writeops\n"); } @@ -203,6 +204,8 @@ TEST(sample_file, write_ops_on_writable) { // check result -- it must contain these two section names CHECK(strstr(buf, ACL_PKG_SECTION_HASH)); CHECK(strstr(buf, ACL_PKG_SECTION_ACL_VERSION)); + + close_file(pkg); printf("end writeops_write\n"); }