lib20/misc: Small robustness improvements to vb2_load_fw_keyblock()

This patch fixes an issue discovered while fuzzing
vb2_load_fw_keyblock(): the data key contained in the keyblock is not
sanity-checked before moving it around on the work buffer, resulting in
a potential overflow if it's key_size flows over the end of the
keyblock. This is not exploitable since the keyblock was already
verified, so only signed (=trusted) keyblocks can get to this stage, but
there's nothing wrong with double-checking anyway.

This patch also rewrites the data_key moving code a bit to just move the
whole key rather than individually copying the header elements and then
just memmove()ing the data (and keeping the previous key_offset from the
root key rather than the one from the data key). None of these issues
affect correctness but it seems simpler and cleaner to me this way.

Finally, remove an instance where the keyblock was accessed after the
memmove(). This would be bad if the data key was so much larger than the
keyblock that memmove()ing it overwrites the keyblock header. Like an
existing comment points out, that doesn't happen with the key sizes we
choose in practice, but it's still better to not rely on that.

BRANCH=none
BUG=chromium:1017793
TEST=make runtests and reran failing fuzz testcase

Change-Id: I78ded43ad999e0883a69cbb2ea7e876888a9fa22
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/1880015
Reviewed-by: Joel Kitching <kitching@chromium.org>
1 file changed