mirror of
https://github.com/Telecominfraproject/OpenCellular.git
synced 2025-11-26 19:25:02 +00:00
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 Change-Id: I469dda19b4589e484a41ca9bae1e107787f3cf4d Reviewed-on: https://gerrit.chromium.org/gerrit/11516 Tested-by: Bill Richardson <wfrichar@chromium.org> Reviewed-by: Gaurav Shah <gauravsh@chromium.org>
This commit is contained in:
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user