diff options
author | Stefan Reinauer <reinauer@chromium.org> | 2011-12-19 15:19:36 -0800 |
---|---|---|
committer | Stefan Reinauer <reinauer@chromium.org> | 2011-12-19 15:48:19 -0800 |
commit | 1c9f7e173fd3a08b22004a74b45dabcfe297aa74 (patch) | |
tree | f69a72980fb9fea1f1dd63800e07bf46d77e48c0 | |
parent | 5c8bfb5ee454c01e60e953c9a4de7ced6a43ff76 (diff) | |
download | vboot-1c9f7e173fd3a08b22004a74b45dabcfe297aa74.tar.gz |
Address security concerns for vboot_audio.c
Based on the compile-time constants, I don't think we were in any danger,
but I've added the checks anyway. It never hurts to be certain!
BUG=chromium-os:22786
TEST=none
Reviewed-on: https://gerrit.chromium.org/gerrit/11516
Tested-by: Bill Richardson <wfrichar@chromium.org>
Reviewed-by: Gaurav Shah <gauravsh@chromium.org>
(cherry picked from commit 931728a003a417bcb29cc1c203ce36d23feee9e8)
Change-Id: I1b6f3bec9e7126d546d1a0102bacab2122a6d538
Reviewed-on: https://gerrit.chromium.org/gerrit/13187
Tested-by: Stefan Reinauer <reinauer@chromium.org>
Reviewed-by: Randall Spangler <rspangler@chromium.org>
-rw-r--r-- | firmware/lib/vboot_audio.c | 27 |
1 files changed, 25 insertions, 2 deletions
diff --git a/firmware/lib/vboot_audio.c b/firmware/lib/vboot_audio.c index b689bbd4..8841a5e7 100644 --- a/firmware/lib/vboot_audio.c +++ b/firmware/lib/vboot_audio.c @@ -13,6 +13,10 @@ #include "vboot_audio_private.h" #include "vboot_common.h" +/* BIOS doesn't have /usr/include */ +#ifndef UINT_MAX +#define UINT_MAX 4294967295U /* 0xffffffff */ +#endif #define DEV_LOOP_TIME 10 /* Minimum note granularity in msecs */ @@ -47,7 +51,7 @@ static void VbGetDevMusicNotes(VbAudioContext *audio, int use_short) { VbDevMusicNote *notebuf = 0; VbDevMusicNote *builtin = 0; VbDevMusic *hdr = CUSTOM_MUSIC_NOTES; - uint32_t maxsize = CUSTOM_MUSIC_MAXSIZE; + uint32_t maxsize = CUSTOM_MUSIC_MAXSIZE; /* always <= flash size (8M) */ uint32_t maxnotes, mysum, mylen, i; uint64_t on_loops, total_loops, min_loops; uint32_t this_loops; @@ -77,6 +81,9 @@ static void VbGetDevMusicNotes(VbAudioContext *audio, int use_short) { goto nope; } + /* How many notes will fit in the flash region? One more than you'd think, + * because there's one note in the header itself. + */ maxnotes = 1 + (maxsize - sizeof(VbDevMusic)) / sizeof(VbDevMusicNote); if (hdr->count == 0 || hdr->count > maxnotes) { VBDEBUG(("VbGetDevMusicNotes: count=%d maxnotes=%d\n", @@ -84,6 +91,16 @@ static void VbGetDevMusicNotes(VbAudioContext *audio, int use_short) { goto nope; } + /* CUSTOM_MUSIC_MAXSIZE can't be larger than the size of the flash (around 8M + * or so) so this isn't really necessary, but let's be safe anyway. + */ + if ((sizeof(VbDevMusicNote) > UINT_MAX / hdr->count) || + (sizeof(hdr->count) > UINT_MAX - hdr->count * sizeof(VbDevMusicNote))) { + VBDEBUG(("VbGetDevMusicNotes: count=%d, just isn't right\n")); + goto nope; + } + + /* Now we know this won't overflow */ mylen = (uint32_t)(sizeof(hdr->count) + hdr->count * sizeof(VbDevMusicNote)); mysum = Crc32(&(hdr->count), mylen); @@ -128,7 +145,13 @@ static void VbGetDevMusicNotes(VbAudioContext *audio, int use_short) { goto nope; } - /* Okay, it looks good. Allocate the space (plus one) and copy it over */ + /* One more check, just to be paranoid. */ + if (hdr->count > (UINT_MAX / sizeof(VbDevMusicNote) - 1)) { + VBDEBUG(("VbGetDevMusicNotes: they're all out to get me!\n")); + goto nope; + } + + /* Okay, it looks good. Allocate the space (plus one) and copy it over. */ notebuf = VbExMalloc((hdr->count + 1) * sizeof(VbDevMusicNote)); Memcpy(notebuf, hdr->notes, hdr->count * sizeof(VbDevMusicNote)); count = hdr->count; |