From 2145721c3c1840561030b27d2207006b0139c16c Mon Sep 17 00:00:00 2001 From: Randall Spangler Date: Fri, 6 Jun 2014 09:30:14 -0700 Subject: vboot2: Use more specific error codes, part 2 Error codes reported by the aligment checks in common.c are now very specific, and tests verify the proper errors are reported. Changed args to vb2_member_inside() so I can force wraparounds. BUG=chromium:370082 BRANCH=none TEST=make clean && VBOOT2=1 COV=1 make Change-Id: Ib135674e82005b76bce7a83a1f4a65a9c5296cf4 Signed-off-by: Randall Spangler Reviewed-on: https://chromium-review.googlesource.com/202937 Reviewed-by: Daisuke Nojiri --- firmware/2lib/2common.c | 27 +++++---- firmware/2lib/include/2common.h | 8 +-- firmware/2lib/include/2return_codes.h | 26 +++++++++ tests/vb2_common_tests.c | 107 +++++++++++++++++++--------------- 4 files changed, 103 insertions(+), 65 deletions(-) diff --git a/firmware/2lib/2common.c b/firmware/2lib/2common.c index 9099f6f7..56593aae 100644 --- a/firmware/2lib/2common.c +++ b/firmware/2lib/2common.c @@ -20,14 +20,14 @@ int vb2_align(uint8_t **ptr, uint32_t *size, uint32_t align, uint32_t want_size) offs = align - offs; if (*size < offs) - return VB2_ERROR_BUFFER_TOO_SMALL; + return VB2_ERROR_ALIGN_BIGGER_THAN_SIZE; *ptr += offs; *size -= offs; } if (*size < want_size) - return VB2_ERROR_BUFFER_TOO_SMALL; + return VB2_ERROR_ALIGN_SIZE; return VB2_SUCCESS; } @@ -106,10 +106,10 @@ ptrdiff_t vb2_offset_of(const void *base, const void *ptr) return (uintptr_t)ptr - (uintptr_t)base; } -int vb2_verify_member_inside(const void *parent, uint32_t parent_size, - const void *member, uint32_t member_size, - uint32_t member_data_offset, - uint32_t member_data_size) +int vb2_verify_member_inside(const void *parent, size_t parent_size, + const void *member, size_t member_size, + ptrdiff_t member_data_offset, + size_t member_data_size) { const size_t psize = (size_t)parent_size; const uintptr_t parent_end = (uintptr_t)parent + parent_size; @@ -120,23 +120,22 @@ int vb2_verify_member_inside(const void *parent, uint32_t parent_size, /* Make sure parent doesn't wrap */ if (parent_end < (uintptr_t)parent) - return VB2_ERROR_UNKNOWN; + return VB2_ERROR_INSIDE_PARENT_WRAPS; /* * Make sure the member is fully contained in the parent and doesn't * wrap. Use >, not >=, since member_size = 0 is possible. */ if (member_end_offs < member_offs) - return VB2_ERROR_UNKNOWN; - if (member_offs > psize || member_end_offs > psize) - return VB2_ERROR_UNKNOWN; - + return VB2_ERROR_INSIDE_MEMBER_WRAPS; + if (member_offs < 0 || member_offs > psize || member_end_offs > psize) + return VB2_ERROR_INSIDE_MEMBER_OUTSIDE; /* Make sure parent fully contains member data */ if (data_end_offs < data_offs) - return VB2_ERROR_UNKNOWN; - if (data_offs > psize || data_end_offs > psize) - return VB2_ERROR_UNKNOWN; + return VB2_ERROR_INSIDE_DATA_WRAPS; + if (data_offs < 0 || data_offs > psize || data_end_offs > psize) + return VB2_ERROR_INSIDE_DATA_OUTSIDE; return VB2_SUCCESS; } diff --git a/firmware/2lib/include/2common.h b/firmware/2lib/include/2common.h index 3bb403be..326f8919 100644 --- a/firmware/2lib/include/2common.h +++ b/firmware/2lib/include/2common.h @@ -151,10 +151,10 @@ uint8_t *vb2_signature_data(struct vb2_signature *sig); * @param member_data_size Size of member data in bytes * @return VB2_SUCCESS, or non-zero if error. */ -int vb2_verify_member_inside(const void *parent, uint32_t parent_size, - const void *member, uint32_t member_size, - uint32_t member_data_offset, - uint32_t member_data_size); +int vb2_verify_member_inside(const void *parent, size_t parent_size, + const void *member, size_t member_size, + ptrdiff_t member_data_offset, + size_t member_data_size); /** * Verify a signature is fully contained in its parent data diff --git a/firmware/2lib/include/2return_codes.h b/firmware/2lib/include/2return_codes.h index 28c0f91d..f81dc732 100644 --- a/firmware/2lib/include/2return_codes.h +++ b/firmware/2lib/include/2return_codes.h @@ -99,6 +99,32 @@ enum vb2_return_code { /* Invalid flags passed to vb2_secdata_set() */ VB2_ERROR_SECDATA_SET_FLAGS, + /********************************************************************** + * Common code errors + */ + VB2_ERROR_COMMON = VB2_ERROR_BASE + 0x050000, + + /* Buffer is smaller than alignment offset in vb2_align() */ + VB2_ERROR_ALIGN_BIGGER_THAN_SIZE, + + /* Buffer is smaller than request in vb2_align() */ + VB2_ERROR_ALIGN_SIZE, + + /* Parent wraps around in vb2_verify_member_inside() */ + VB2_ERROR_INSIDE_PARENT_WRAPS, + + /* Member wraps around in vb2_verify_member_inside() */ + VB2_ERROR_INSIDE_MEMBER_WRAPS, + + /* Member outside parent in vb2_verify_member_inside() */ + VB2_ERROR_INSIDE_MEMBER_OUTSIDE, + + /* Member data wraps around in vb2_verify_member_inside() */ + VB2_ERROR_INSIDE_DATA_WRAPS, + + /* Member data outside parent in vb2_verify_member_inside() */ + VB2_ERROR_INSIDE_DATA_OUTSIDE, + /********************************************************************** * TODO: errors which must still be made specific */ diff --git a/tests/vb2_common_tests.c b/tests/vb2_common_tests.c index fd8a7acc..e476edf1 100644 --- a/tests/vb2_common_tests.c +++ b/tests/vb2_common_tests.c @@ -23,27 +23,30 @@ static void test_align(void) p0 = (uint8_t *)buf; ptr = p0; size = 16; - TEST_EQ(vb2_align(&ptr, &size, 4, 16), 0, "vb2_align() aligned"); + TEST_SUCC(vb2_align(&ptr, &size, 4, 16), "vb2_align() aligned"); TEST_EQ(vb2_offset_of(p0, ptr), 0, "ptr"); TEST_EQ(size, 16, " size"); - TEST_NEQ(vb2_align(&ptr, &size, 4, 17), 0, "vb2_align() small"); + TEST_EQ(vb2_align(&ptr, &size, 4, 17), + VB2_ERROR_ALIGN_SIZE, "vb2_align() small"); /* Offset */ ptr = p0 + 1; size = 15; - TEST_EQ(vb2_align(&ptr, &size, 4, 12), 0, "vb2_align() offset"); + TEST_SUCC(vb2_align(&ptr, &size, 4, 12), "vb2_align() offset"); TEST_EQ(vb2_offset_of(p0, ptr), 4, "ptr"); TEST_EQ(size, 12, " size"); /* Offset, now too small */ ptr = p0 + 1; size = 15; - TEST_NEQ(vb2_align(&ptr, &size, 4, 15), 0, "vb2_align() offset small"); + TEST_EQ(vb2_align(&ptr, &size, 4, 15), + VB2_ERROR_ALIGN_SIZE, "vb2_align() offset small"); /* Offset, too small even to align */ ptr = p0 + 1; size = 1; - TEST_NEQ(vb2_align(&ptr, &size, 4, 1), 0, "vb2_align() offset tiny"); + TEST_EQ(vb2_align(&ptr, &size, 4, 1), + VB2_ERROR_ALIGN_BIGGER_THAN_SIZE, "vb2_align() offset tiny"); } /** @@ -149,69 +152,79 @@ static void test_helper_functions(void) { uint8_t *p = (uint8_t *)test_helper_functions; - TEST_EQ(vb2_verify_member_inside(p, 20, p, 6, 11, 3), 0, - "MemberInside ok 1"); - TEST_EQ(vb2_verify_member_inside(p, 20, p+4, 4, 8, 4), 0, - "MemberInside ok 2"); - TEST_NEQ(vb2_verify_member_inside(p, 20, p-4, 4, 8, 4), 0, - "MemberInside member before parent"); - TEST_NEQ(vb2_verify_member_inside(p, 20, p+20, 4, 8, 4), 0, - "MemberInside member after parent"); - TEST_NEQ(vb2_verify_member_inside(p, 20, p, 21, 0, 0), 0, - "MemberInside member too big"); - TEST_NEQ(vb2_verify_member_inside(p, 20, p, 4, 21, 0), 0, - "MemberInside data after parent"); - TEST_NEQ(vb2_verify_member_inside(p, 20, p, 4, UINT32_MAX, 0), - 0, "MemberInside data before parent"); - TEST_NEQ(vb2_verify_member_inside(p, 20, p, 4, 4, 17), 0, - "MemberInside data too big"); - TEST_NEQ(vb2_verify_member_inside(p, UINT32_MAX, - p+UINT32_MAX-9, 12, 5, 0), 0, - "MemberInside wraparound 1"); - TEST_NEQ(vb2_verify_member_inside(p, UINT32_MAX, - p+UINT32_MAX-9, 5, 12, 0), 0, - "MemberInside wraparound 2"); - TEST_NEQ(vb2_verify_member_inside(p, UINT32_MAX, - p+UINT32_MAX-9, 5, 0, 12), 0, - "MemberInside wraparound 3"); + TEST_SUCC(vb2_verify_member_inside(p, 20, p, 6, 11, 3), + "MemberInside ok 1"); + TEST_SUCC(vb2_verify_member_inside(p, 20, p+4, 4, 8, 4), + "MemberInside ok 2"); + TEST_EQ(vb2_verify_member_inside(p, 20, p-4, 4, 8, 4), + VB2_ERROR_INSIDE_MEMBER_OUTSIDE, + "MemberInside member before parent"); + TEST_EQ(vb2_verify_member_inside(p, 20, p+20, 4, 8, 4), + VB2_ERROR_INSIDE_MEMBER_OUTSIDE, + "MemberInside member after parent"); + TEST_EQ(vb2_verify_member_inside(p, 20, p, 21, 0, 0), + VB2_ERROR_INSIDE_MEMBER_OUTSIDE, + "MemberInside member too big"); + TEST_EQ(vb2_verify_member_inside(p, 20, p, 4, 21, 0), + VB2_ERROR_INSIDE_DATA_OUTSIDE, + "MemberInside data after parent"); + TEST_EQ(vb2_verify_member_inside(p, 20, p, 4, SIZE_MAX, 0), + VB2_ERROR_INSIDE_DATA_OUTSIDE, + "MemberInside data before parent"); + TEST_EQ(vb2_verify_member_inside(p, 20, p, 4, 4, 17), + VB2_ERROR_INSIDE_DATA_OUTSIDE, + "MemberInside data too big"); + TEST_EQ(vb2_verify_member_inside(p, -8, p, 12, 0, 0), + VB2_ERROR_INSIDE_PARENT_WRAPS, + "MemberInside wraparound 1"); + TEST_EQ(vb2_verify_member_inside(p, 20, p, -8, 0, 0), + VB2_ERROR_INSIDE_MEMBER_WRAPS, + "MemberInside wraparound 2"); + TEST_EQ(vb2_verify_member_inside(p, 20, p, 4, 0, -8), + VB2_ERROR_INSIDE_DATA_WRAPS, + "MemberInside wraparound 3"); } { struct vb2_packed_key k = {.key_offset = sizeof(k), .key_size = 128}; - TEST_EQ(vb2_verify_packed_key_inside(&k, sizeof(k)+128, &k), 0, - "PublicKeyInside ok 1"); - TEST_EQ(vb2_verify_packed_key_inside(&k - 1, - 2*sizeof(k)+128, &k), - 0, "PublicKeyInside ok 2"); - TEST_NEQ(vb2_verify_packed_key_inside(&k, 128, &k), 0, - "PublicKeyInside key too big"); + TEST_SUCC(vb2_verify_packed_key_inside(&k, sizeof(k)+128, &k), + "PublicKeyInside ok 1"); + TEST_SUCC(vb2_verify_packed_key_inside(&k - 1, + 2*sizeof(k)+128, &k), + "PublicKeyInside ok 2"); + TEST_EQ(vb2_verify_packed_key_inside(&k, 128, &k), + VB2_ERROR_INSIDE_DATA_OUTSIDE, + "PublicKeyInside key too big"); } { struct vb2_packed_key k = {.key_offset = 100, .key_size = 4}; - TEST_NEQ(vb2_verify_packed_key_inside(&k, 99, &k), 0, - "PublicKeyInside offset too big"); + TEST_EQ(vb2_verify_packed_key_inside(&k, 99, &k), + VB2_ERROR_INSIDE_DATA_OUTSIDE, + "PublicKeyInside offset too big"); } { struct vb2_signature s = {.sig_offset = sizeof(s), .sig_size = 128}; - TEST_EQ(vb2_verify_signature_inside(&s, sizeof(s)+128, &s), 0, + TEST_SUCC(vb2_verify_signature_inside(&s, sizeof(s)+128, &s), "SignatureInside ok 1"); - TEST_EQ(vb2_verify_signature_inside(&s - 1, - 2*sizeof(s)+128, &s), - 0, "SignatureInside ok 2"); - TEST_NEQ(vb2_verify_signature_inside(&s, 128, &s), 0, - "SignatureInside sig too big"); + TEST_SUCC(vb2_verify_signature_inside(&s - 1, + 2*sizeof(s)+128, &s), + "SignatureInside ok 2"); + TEST_EQ(vb2_verify_signature_inside(&s, 128, &s), + VB2_ERROR_INSIDE_DATA_OUTSIDE, + "SignatureInside sig too big"); } { struct vb2_signature s = {.sig_offset = 100, .sig_size = 4}; - TEST_NEQ(vb2_verify_signature_inside(&s, 99, &s), 0, - "SignatureInside offset too big"); + TEST_EQ(vb2_verify_signature_inside(&s, 99, &s), + VB2_ERROR_INSIDE_DATA_OUTSIDE, + "SignatureInside offset too big"); } } -- cgit v1.2.1