diff options
author | Randall Spangler <rspangler@chromium.org> | 2014-10-23 17:04:03 -0700 |
---|---|---|
committer | chrome-internal-fetch <chrome-internal-fetch@google.com> | 2014-10-29 22:23:43 +0000 |
commit | f6cfb974ce465cf977490fe26db9c8735da97571 (patch) | |
tree | 2de99fe91415267d6020d56ac28fc6c79d4e5d2c | |
parent | 7c2beb08380410ca6847abdac23e11ded2d1b625 (diff) | |
download | vboot-f6cfb974ce465cf977490fe26db9c8735da97571.tar.gz |
vboot2: Add verification for common vb2 struct header
All new-style structs have a common header. This adds a verification
function for that common header, and tests for it.
BUG=chromium:423882
BRANCH=none
TEST=VBOOT2=1 make runtests
Change-Id: I668486e77f7200c10b43aa2d17b4dd6639e5538e
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/225459
Reviewed-by: Bill Richardson <wfrichar@chromium.org>
-rw-r--r-- | firmware/2lib/2common.c | 47 | ||||
-rw-r--r-- | firmware/2lib/include/2common.h | 14 | ||||
-rw-r--r-- | firmware/2lib/include/2return_codes.h | 9 | ||||
-rw-r--r-- | firmware/2lib/include/2struct.h | 10 | ||||
-rw-r--r-- | tests/vb2_common_tests.c | 46 |
5 files changed, 115 insertions, 11 deletions
diff --git a/firmware/2lib/2common.c b/firmware/2lib/2common.c index 872b6233..6360ef09 100644 --- a/firmware/2lib/2common.c +++ b/firmware/2lib/2common.c @@ -125,7 +125,6 @@ int vb2_verify_member_inside(const void *parent, size_t parent_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; const ptrdiff_t member_offs = vb2_offset_of(parent, member); const ptrdiff_t member_end_offs = member_offs + member_size; @@ -133,27 +132,61 @@ int vb2_verify_member_inside(const void *parent, size_t parent_size, const ptrdiff_t data_end_offs = data_offs + member_data_size; /* Make sure parent doesn't wrap */ - if (parent_end < (uintptr_t)parent) + if (parent_size < 0 || parent_end < (uintptr_t)parent) 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) + if (member_size < 0 || member_end_offs < member_offs) return VB2_ERROR_INSIDE_MEMBER_WRAPS; - if (member_offs < 0 || member_offs > psize || member_end_offs > psize) + if (member_offs < 0 || member_offs > parent_size || + member_end_offs > parent_size) return VB2_ERROR_INSIDE_MEMBER_OUTSIDE; - /* Make sure parent fully contains member data */ - if (data_end_offs < data_offs) + /* Make sure the member data is after the member */ + if (member_data_size > 0 && data_offs < member_end_offs) + return VB2_ERROR_INSIDE_DATA_OVERLAP; + + /* Make sure parent fully contains member data, if any */ + if (member_data_size < 0 || data_end_offs < data_offs) return VB2_ERROR_INSIDE_DATA_WRAPS; - if (data_offs < 0 || data_offs > psize || data_end_offs > psize) + if (data_offs < 0 || data_offs > parent_size || + data_end_offs > parent_size) return VB2_ERROR_INSIDE_DATA_OUTSIDE; return VB2_SUCCESS; } +int vb2_verify_common_header(const void *parent, + uint32_t parent_size, + const struct vb2_struct_common *c) +{ + int rv; + + /* Make sure common data and description are inside parent */ + rv = vb2_verify_member_inside(parent, parent_size, + c, sizeof(*c), + c->desc_offset, c->desc_size); + if (rv) + return rv; + + /* Check description */ + if (c->desc_size > 0) { + /* Description must be null-terminated */ + const uint8_t *cptr = (const uint8_t *)c; + if (cptr[c->desc_offset + c->desc_size - 1] != 0) + return VB2_ERROR_DESC_TERMINATOR; + + } else if (c->desc_offset != 0) { + /* If size is non-zero, offset must be too */ + return VB2_ERROR_DESC_EMPTY_OFFSET; + } + + return VB2_SUCCESS; +} + int vb2_verify_signature_inside(const void *parent, uint32_t parent_size, const struct vb2_signature *sig) diff --git a/firmware/2lib/include/2common.h b/firmware/2lib/include/2common.h index dcf799a0..8a00dd71 100644 --- a/firmware/2lib/include/2common.h +++ b/firmware/2lib/include/2common.h @@ -181,6 +181,20 @@ int vb2_verify_member_inside(const void *parent, size_t parent_size, size_t member_data_size); /** + * Verify the common struct header is fully contained in its parent data + * + * Also verifies the description is either zero-length or null-terminated. + * + * @param parent Parent data + * @param parent_size Parent size in bytes + * @param sig Signature pointer + * @return VB2_SUCCESS, or non-zero if error. + */ +int vb2_verify_common_header(const void *parent, + uint32_t parent_size, + const struct vb2_struct_common *c); + +/** * Verify a signature is fully contained in its parent data * * @param parent Parent data diff --git a/firmware/2lib/include/2return_codes.h b/firmware/2lib/include/2return_codes.h index ee6c91fe..333c29c1 100644 --- a/firmware/2lib/include/2return_codes.h +++ b/firmware/2lib/include/2return_codes.h @@ -167,6 +167,15 @@ enum vb2_return_code { /* Unsupported hash algorithm in vb2_unpack_key() */ VB2_ERROR_UNPACK_KEY_HASH_ALGORITHM, + /* Common struct description is not null-terminated */ + VB2_ERROR_DESC_TERMINATOR, + + /* Common struct description offset is empty, but offset is non-zero */ + VB2_ERROR_DESC_EMPTY_OFFSET, + + /* Member data overlaps member header */ + VB2_ERROR_INSIDE_DATA_OVERLAP, + /********************************************************************** * Keyblock verification errors (all in vb2_verify_keyblock()) */ diff --git a/firmware/2lib/include/2struct.h b/firmware/2lib/include/2struct.h index 38153c5c..12e6e0b8 100644 --- a/firmware/2lib/include/2struct.h +++ b/firmware/2lib/include/2struct.h @@ -277,12 +277,16 @@ struct vb2_struct_common { uint16_t struct_version_major; uint16_t struct_version_minor; - /* Offset of null-terminated ASCII description from start of struct */ + /* + * Offset of null-terminated ASCII description from start of struct. + * Must be 0 if desc_size=0. + */ uint32_t desc_offset; /* - * Size of description in bytes, counting null terminator. May be 0 - * if no description is present. + * Size of description in bytes, counting null terminator. Set 0 if no + * description is present. If non-zero, there must be a null + * terminator (0) at offset (desc_offset + desc_size - 1). */ uint32_t desc_size; } __attribute__((packed)); diff --git a/tests/vb2_common_tests.c b/tests/vb2_common_tests.c index c70de2ea..5a8cb202 100644 --- a/tests/vb2_common_tests.c +++ b/tests/vb2_common_tests.c @@ -228,18 +228,62 @@ static void test_helper_functions(void) 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, 20, p, 8, 4, 8), + VB2_ERROR_INSIDE_DATA_OVERLAP, + "MemberInside data overlaps member"); 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), + TEST_EQ(vb2_verify_member_inside(p, 20, p, 4, 4, -12), VB2_ERROR_INSIDE_DATA_WRAPS, "MemberInside wraparound 3"); } { + uint8_t cbuf[sizeof(struct vb2_struct_common) + 128]; + struct vb2_struct_common *c = (struct vb2_struct_common *)cbuf; + + c->desc_offset = sizeof(*c); + c->desc_size = 128; + cbuf[sizeof(cbuf) - 1] = 0; + TEST_SUCC(vb2_verify_common_header(cbuf, sizeof(cbuf), c), + "CommonInside at start"); + + c[1].desc_offset = sizeof(*c); + c[1].desc_size = 128 - sizeof(*c); + TEST_SUCC(vb2_verify_common_header(cbuf, sizeof(cbuf), c + 1), + "CommonInside after start"); + + TEST_EQ(vb2_verify_common_header(cbuf, sizeof(cbuf) - 1, c), + VB2_ERROR_INSIDE_DATA_OUTSIDE, + "CommonInside key too big"); + + c->desc_offset = sizeof(cbuf); + TEST_EQ(vb2_verify_common_header(cbuf, sizeof(cbuf), c), + VB2_ERROR_INSIDE_DATA_OUTSIDE, + "CommonInside offset too big"); + c->desc_offset = sizeof(*c); + + cbuf[sizeof(cbuf) - 1] = 1; + TEST_EQ(vb2_verify_common_header(cbuf, sizeof(cbuf), c), + VB2_ERROR_DESC_TERMINATOR, + "CommonInside description not terminated"); + + c->desc_size = 0; + c->desc_offset = 0; + TEST_SUCC(vb2_verify_common_header(cbuf, sizeof(cbuf), c), + "CommonInside no description"); + + c->desc_offset = 4; + TEST_EQ(vb2_verify_common_header(cbuf, sizeof(cbuf), c), + VB2_ERROR_DESC_EMPTY_OFFSET, + "CommonInside description empty offset"); + } + + { struct vb2_packed_key k = {.key_offset = sizeof(k), .key_size = 128}; TEST_SUCC(vb2_verify_packed_key_inside(&k, sizeof(k)+128, &k), |