img-verifier: Fix build error due to overflow by read 82/319882/2
authorUnsung Lee <unsung.lee@samsung.com>
Wed, 19 Feb 2025 05:05:09 +0000 (14:05 +0900)
committerUnsung Lee <unsung.lee@samsung.com>
Wed, 19 Feb 2025 07:37:12 +0000 (16:37 +0900)
Fix build error when Werror=stringop-overflow flag is turned on.
This flag returns build error if read buffer size is smaller than read size.
To solve this problem, declare a separate structure that manages only the metadata of signed_file.
Then, change it so that the address of this structure is passed to the read function.

Change-Id: I88e18ec19ac71f538b1300d0028b29563bcd58b7
Signed-off-by: Unsung Lee <unsung.lee@samsung.com>
src/img-verifier/img-verifier.c
src/img-verifier/img-verifier.h

index 8c56bf66527bbc1e91da536332ae2ad4474189d2..06a9ef864b4bf75a59ef908edeca20527f6f8094 100644 (file)
@@ -20,7 +20,10 @@ static EVP_MD_CTX *evp_md_ctx;
 
 static bool check_signed_file(const char *path)
 {
-       const int metadata_size = sizeof(signed_file.delta_size) + sizeof(signed_file.signature_size) + sizeof(signed_file.certificate_size) + MAGIC_NUMBER_SIZE;
+       const int metadata_size =
+               sizeof(signed_file.metadata.delta_size) +
+               sizeof(signed_file.metadata.signature_size) +
+               sizeof(signed_file.metadata.certificate_size) + MAGIC_NUMBER_SIZE;
        int ret = -1;
        _CLEANUP_CLOSE_ int signed_file_fd = -1;
        long long int offset = (off_t)-1;
@@ -39,21 +42,23 @@ static bool check_signed_file(const char *path)
        ASSERT_RETV(offset != (off_t)-1, false, "Failed to move file offset : %m");
 
        // read also fills other members of signed_file
-       ret = read(signed_file_fd, &signed_file.delta_size, metadata_size);
+       ret = read(signed_file_fd, &signed_file.metadata, metadata_size);
        ASSERT_RETV(ret == metadata_size, false, "Failed to read metadata : %m");
 
        // Check magic number
-       _D("Magic number : %s", signed_file.magic_number);
+       _D("Magic number : %s", signed_file.metadata.magic_number);
 
-       ret = memcmp(signed_file.magic_number, MAGIC_NUMBER, MAGIC_NUMBER_SIZE);
+       ret = memcmp(signed_file.metadata.magic_number, MAGIC_NUMBER, MAGIC_NUMBER_SIZE);
        ASSERT_RETV(ret == 0, false, "Invalid magic number");
 
        // Check file size
-       _D("Delta size : %d", signed_file.delta_size);
-       _D("Signature size : %d", signed_file.signature_size);
-       _D("Certificate size : %d", signed_file.certificate_size);
+       _D("Delta size : %d", signed_file.metadata.delta_size);
+       _D("Signature size : %d", signed_file.metadata.signature_size);
+       _D("Certificate size : %d", signed_file.metadata.certificate_size);
 
-       data_size = signed_file.delta_size + signed_file.signature_size + signed_file.certificate_size;
+       data_size = signed_file.metadata.delta_size +
+               signed_file.metadata.signature_size +
+               signed_file.metadata.certificate_size;
 
        offset = lseek(signed_file_fd, -metadata_size, SEEK_END);
        ASSERT_RETV(offset == data_size, false, "Invalid file size : Expected(%u), Real(%lld)", data_size, offset);
@@ -118,20 +123,22 @@ static int read_signed_file(const char *path)
        ASSERT_RETV(signed_file_fd != -1, errno, "Failed to open %s : %m", path);
 
        // Read data
-       offset = lseek(signed_file_fd, signed_file.delta_size, SEEK_SET);
+       offset = lseek(signed_file_fd, signed_file.metadata.delta_size, SEEK_SET);
        ASSERT_RETV(offset != (off_t)-1, false, "Failed to move file offset : %m");
 
-       signed_file.signature = malloc(signed_file.signature_size);
+       signed_file.signature = malloc(signed_file.metadata.signature_size);
        ASSERT_RETV(signed_file.signature, ENOMEM, "Not enough memory");
 
-       ret = read(signed_file_fd, signed_file.signature, signed_file.signature_size);
-       ASSERT_RETV(ret >= 0 && (unsigned int)ret == signed_file.signature_size, errno, "Failed to read signature : %m");
+       ret = read(signed_file_fd, signed_file.signature, signed_file.metadata.signature_size);
+       ASSERT_RETV(ret >= 0 && (unsigned int)ret == signed_file.metadata.signature_size,
+                       errno, "Failed to read signature : %m");
 
-       signed_file.certificate = malloc(signed_file.certificate_size);
+       signed_file.certificate = malloc(signed_file.metadata.certificate_size);
        ASSERT_RETV(signed_file.certificate, ENOMEM, "Not enough memory");
 
-       ret = read(signed_file_fd, signed_file.certificate, signed_file.certificate_size);
-       ASSERT_RETV(ret >= 0 && (unsigned int)ret == signed_file.certificate_size, errno, "Failed to read certificate : %m");
+       ret = read(signed_file_fd, signed_file.certificate, signed_file.metadata.certificate_size);
+       ASSERT_RETV(ret >= 0 && (unsigned int)ret == signed_file.metadata.certificate_size,
+                       errno, "Failed to read certificate : %m");
 
        return SUCCEED;
 }
@@ -149,7 +156,8 @@ static int verify_certificate(void)
        pcert = signed_file.certificate;
        ASSERT_RETV(pcert, EINVAL, "Invalid certificate");
 
-       signed_file_cert = d2i_X509(NULL, (const unsigned char **)&pcert, signed_file.certificate_size);
+       signed_file_cert = d2i_X509(NULL, (const unsigned char **)&pcert,
+                       signed_file.metadata.certificate_size);
        ASSERT_RETV(signed_file_cert, print_openssl_error(), "d2i_X509 failed");
 
        // Reject the certificate same with CA
@@ -214,7 +222,7 @@ static int verify_delta(const char *path)
        signed_file_fd = open(path, O_RDONLY);
        ASSERT_RETV(signed_file_fd != -1, false, "Failed to open %s : %m", path);
 
-       while (read_delta_size < signed_file.delta_size) {
+       while (read_delta_size < signed_file.metadata.delta_size) {
                memset(delta_block, 0, sizeof(delta_block));
                ret = read(signed_file_fd, delta_block, sizeof(delta_block));
                ASSERT_RETV(ret != -1, errno, "Failed to read delta : %m");
@@ -225,7 +233,7 @@ static int verify_delta(const char *path)
        }
 
        // Decrypt signature and compare with hashed delta
-       ret = EVP_DigestVerifyFinal(evp_md_ctx, signed_file.signature, signed_file.signature_size);
+       ret = EVP_DigestVerifyFinal(evp_md_ctx, signed_file.signature, signed_file.metadata.signature_size);
        ASSERT_RETV(ret == 1, print_openssl_error(), "EVP_DigestVerifyFinal failed");
 
        _I("Verify result : VALID");
index aa4272bef6f7c7d7014d6266e78259b0036c0541..058d13dd56f44ef376a4a3e5b9fa7b1c4978b866 100644 (file)
@@ -45,6 +45,13 @@ static void close_dir(DIR **dirp)
 #define MAGIC_NUMBER "IMG_SIGNED_V1"
 #define MAGIC_NUMBER_SIZE (sizeof(MAGIC_NUMBER) - 1)
 
+struct img_verifier_signed_file_metadata {
+       unsigned int delta_size;
+       unsigned int signature_size;
+       unsigned int certificate_size;
+       unsigned char magic_number[MAGIC_NUMBER_SIZE + 1];
+};
+
 /**
  * This structure is same with signed image.
  * So, please sync it with signer.
@@ -54,11 +61,7 @@ struct signed_file {
 //     unsigned char *delta; (not used, just to understand structure)
        unsigned char *signature;
        unsigned char *certificate;
-       // Metadata
-       unsigned int delta_size;
-       unsigned int signature_size;
-       unsigned int certificate_size;
-       unsigned char magic_number[MAGIC_NUMBER_SIZE + 1];
+       struct img_verifier_signed_file_metadata metadata;
 };
 
 #endif /* __IMG_VERIFIER_H__ */