diff options
author | Bill Richardson <wfrichar@chromium.org> | 2010-10-12 07:33:15 -0700 |
---|---|---|
committer | Bill Richardson <wfrichar@chromium.org> | 2010-10-12 07:33:15 -0700 |
commit | c4e92af85ac2bfd90ab82c0b13bb0041595a0aff (patch) | |
tree | 5ff40795b6727f5aedd7a9f4101a76198ca29869 | |
parent | 2845b97df68da9387c400fb1eca5f6dbce1ddefa (diff) | |
download | vboot-c4e92af85ac2bfd90ab82c0b13bb0041595a0aff.tar.gz |
Address some security concerns in the cgpt tool.
1. Check for potential integer overflow in sector_bytes * sector_count.
2. Added O_NOFOLLOW to open() call - Is this enough?
3. Passing buffer length to GuidToStr(), PMBRToStr().
4. Use unsigned int in GetEntry() to determine stride.
5. Address conversion between UTF16 and UTF8.
Note: The UTF conversion is complex and troublesome, and needs careful
consideration to get right. For now, I've just forced the interpretation of
the partition name to 7-bit ASCII. That's sufficient for the needs of Chrome
OS, and I can file a new issue to handle UTF correctly.
BUG=chrome-os-partner:705
TEST=manual
Running "make runtests" invokes the tests/run_cgpt_tests.sh script, which checks the behavior and output of the cgpt tool.
Review URL: http://codereview.chromium.org/3594010
Change-Id: I5fd29796d8c929527e0cfbc6d5ccbcdc77502c6b
-rw-r--r-- | cgpt/cgpt.h | 53 | ||||
-rw-r--r-- | cgpt/cgpt_common.c | 242 | ||||
-rw-r--r-- | cgpt/cmd_add.c | 13 | ||||
-rw-r--r-- | cgpt/cmd_boot.c | 16 | ||||
-rw-r--r-- | cgpt/cmd_create.c | 4 | ||||
-rw-r--r-- | cgpt/cmd_find.c | 9 | ||||
-rw-r--r-- | cgpt/cmd_show.c | 73 | ||||
-rw-r--r-- | utility/vbutil_keyblock.c | 15 |
8 files changed, 210 insertions, 215 deletions
diff --git a/cgpt/cgpt.h b/cgpt/cgpt.h index 1ef74de3..9b0805cf 100644 --- a/cgpt/cgpt.h +++ b/cgpt/cgpt.h @@ -9,6 +9,8 @@ #define _FILE_OFFSET_BITS 64 #include <features.h> #include <stdint.h> +#include <stdio.h> +#include <stdlib.h> #include "endian.h" #include "gpt.h" #include "cgptlib.h" @@ -45,7 +47,7 @@ struct pmbr { uint8_t sig[2]; // 0x55, 0xaa } __attribute__((packed)); -void PMBRToStr(struct pmbr *pmbr, char *str); +void PMBRToStr(struct pmbr *pmbr, char *str, unsigned int buflen); // Handle to the drive storing the GPT. struct drive { @@ -69,7 +71,7 @@ int CheckValid(const struct drive *drive); */ #define GUID_STRLEN 37 int StrToGuid(const char *str, Guid *guid); -void GuidToStr(const Guid *guid, char *str); +void GuidToStr(const Guid *guid, char *str, unsigned int buflen); int IsZero(const Guid *guid); @@ -77,33 +79,33 @@ int ReadPMBR(struct drive *drive); int WritePMBR(struct drive *drive); -/* Convert UTF16 string to UTF8. Rewritten from gpt utility. - * Caller must prepare enough space for UTF8. The rough estimation is: - * - * utf8 length = bytecount(utf16) * 1.5 +/* Convert possibly unterminated UTF16 string to UTF8. + * Caller must prepare enough space for UTF8, which could be up to + * twice the number of UTF16 chars plus the terminating '\0'. */ -void UTF16ToUTF8(const uint16_t *utf16, uint8_t *utf8); -/* Convert UTF8 string to UTF16. Rewritten from gpt utility. - * Caller must prepare enough space for UTF16. The conservative estimation is: - * - * utf16 bytecount = bytecount(utf8) / 3 * 4 +void UTF16ToUTF8(const uint16_t *utf16, unsigned int maxinput, + uint8_t *utf8, unsigned int maxoutput); +/* Convert null-terminated UTF8 string to UTF16. + * Caller must prepare enough space for UTF16, including a terminating 0x0000 */ -void UTF8ToUTF16(const uint8_t *utf8, uint16_t *utf16); +void UTF8ToUTF16(const uint8_t *utf8, uint16_t *utf16, unsigned int maxoutput); /* Helper functions for supported GPT types. */ int ResolveType(const Guid *type, char *buf); int SupportedType(const char *name, Guid *type); void PrintTypes(void); -void EntryDetails(GptEntry *entry, int index, int raw); +void EntryDetails(GptEntry *entry, uint32_t index, int raw); uint32_t GetNumberOfEntries(const GptData *gpt); -GptEntry *GetEntry(GptData *gpt, int secondary, int entry_index); -void SetPriority(GptData *gpt, int secondary, int entry_index, int priority); -int GetPriority(GptData *gpt, int secondary, int entry_index); -void SetTries(GptData *gpt, int secondary, int entry_index, int tries); -int GetTries(GptData *gpt, int secondary, int entry_index); -void SetSuccessful(GptData *gpt, int secondary, int entry_index, int success); -int GetSuccessful(GptData *gpt, int secondary, int entry_index); +GptEntry *GetEntry(GptData *gpt, int secondary, uint32_t entry_index); +void SetPriority(GptData *gpt, int secondary, uint32_t entry_index, + int priority); +int GetPriority(GptData *gpt, int secondary, uint32_t entry_index); +void SetTries(GptData *gpt, int secondary, uint32_t entry_index, int tries); +int GetTries(GptData *gpt, int secondary, uint32_t entry_index); +void SetSuccessful(GptData *gpt, int secondary, uint32_t entry_index, + int success); +int GetSuccessful(GptData *gpt, int secondary, uint32_t entry_index); uint8_t RepairHeader(GptData *gpt, const uint32_t valid_headers); uint8_t RepairEntries(GptData *gpt, const uint32_t valid_entries); @@ -127,5 +129,16 @@ int cmd_find(int argc, char *argv[]); #define ARRAY_COUNT(array) (sizeof(array)/sizeof((array)[0])) const char *GptError(int errnum); +// Size in chars of the GPT Entry's PartitionName field +#define GPT_PARTNAME_LEN 72 + +/* The standard "assert" macro goes away when NDEBUG is defined. This doesn't. + */ +#define require(A) do { \ + if (!(A)) { \ + fprintf(stderr, "condition (%s) failed at %s:%d\n", \ + #A, __FILE__, __LINE__); \ + exit(1); } \ + } while (0) #endif // VBOOT_REFERENCE_UTILITY_CGPT_CGPT_H_ diff --git a/cgpt/cgpt_common.c b/cgpt/cgpt_common.c index 8e8196b4..4b9a5a14 100644 --- a/cgpt/cgpt_common.c +++ b/cgpt/cgpt_common.c @@ -11,6 +11,7 @@ #include <errno.h> #include <fcntl.h> #include <getopt.h> +#include <stdarg.h> #include <stdint.h> #include <stdio.h> #include <stdlib.h> @@ -20,13 +21,10 @@ #include <sys/stat.h> #include <sys/types.h> #include <unistd.h> -#include <assert.h> -#include <stdarg.h> #include "cgptlib_internal.h" #include "crc32.h" - void Error(const char *format, ...) { va_list ap; va_start(ap, format); @@ -65,17 +63,32 @@ static int Load(const int fd, uint8_t **buf, int count; /* byte count to read */ int nread; - assert(buf); + require(buf); + if (!sector_count || !sector_bytes) { + Error("%s() failed at line %d: sector_count=%d, sector_bytes=%d\n", + __FUNCTION__, __LINE__, sector_count, sector_bytes); + return CGPT_FAILED; + } + /* Make sure that sector_bytes * sector_count doesn't roll over. */ + if (sector_bytes > (UINT64_MAX / sector_count)) { + Error("%s() failed at line %d: sector_count=%d, sector_bytes=%d\n", + __FUNCTION__, __LINE__, sector_count, sector_bytes); + return CGPT_FAILED; + } count = sector_bytes * sector_count; *buf = malloc(count); - assert(*buf); + require(*buf); - if (-1 == lseek(fd, sector * sector_bytes, SEEK_SET)) + if (-1 == lseek(fd, sector * sector_bytes, SEEK_SET)) { + Error("Can't lseek: %s\n", strerror(errno)); goto error_free; + } nread = read(fd, *buf, count); - if (nread < count) + if (nread < count) { + Error("Can't read enough: %d, not %d\n", nread, count); goto error_free; + } return CGPT_OK; @@ -125,7 +138,7 @@ static int Save(const int fd, const uint8_t *buf, int count; /* byte count to write */ int nwrote; - assert(buf); + require(buf); count = sector_bytes * sector_count; if (-1 == lseek(fd, sector * sector_bytes, SEEK_SET)) @@ -140,25 +153,26 @@ static int Save(const int fd, const uint8_t *buf, // Opens a block device or file, loads raw GPT data from it. -// +// // Returns CGPT_FAILED if any error happens. // Returns CGPT_OK if success and information are stored in 'drive'. */ int DriveOpen(const char *drive_path, struct drive *drive) { struct stat stat; - assert(drive_path); - assert(drive); + require(drive_path); + require(drive); // Clear struct for proper error handling. memset(drive, 0, sizeof(struct drive)); - drive->fd = open(drive_path, O_RDWR | O_LARGEFILE); + drive->fd = open(drive_path, O_RDWR | O_LARGEFILE | O_NOFOLLOW); if (drive->fd == -1) { Error("Can't open %s: %s\n", drive_path, strerror(errno)); return CGPT_FAILED; } if (fstat(drive->fd, &stat) == -1) { + Error("Can't fstat %s: %s\n", drive_path, strerror(errno)); goto error_close; } if ((stat.st_mode & S_IFMT) != S_IFREG) { @@ -204,7 +218,7 @@ int DriveOpen(const char *drive_path, struct drive *drive) { drive->gpt.sector_bytes, GPT_ENTRIES_SECTORS)) { goto error_close; } - + // We just load the data. Caller must validate it. return CGPT_OK; @@ -226,7 +240,7 @@ int DriveClose(struct drive *drive, int update_as_needed) { Error("Cannot write primary header: %s\n", strerror(errno)); } } - + if (drive->gpt.modified & GPT_MODIFIED_HEADER2) { if(CGPT_OK != Save(drive->fd, drive->gpt.secondary_header, drive->gpt.drive_sectors - GPT_PMBR_SECTOR, @@ -273,7 +287,6 @@ int DriveClose(struct drive *drive, int update_as_needed) { } - /* GUID conversion functions. Accepted format: * * "C12A7328-F81F-11D2-BA4B-00A0C93EC93B" @@ -297,7 +310,7 @@ int StrToGuid(const char *str, Guid *guid) { chunk+7, chunk+8, chunk+9, - chunk+10)) { + chunk+10)) { printf("FAILED\n"); return CGPT_FAILED; } @@ -321,115 +334,72 @@ int StrToGuid(const char *str, Guid *guid) { return CGPT_OK; } -void GuidToStr(const Guid *guid, char *str) { - sprintf(str, "%08X-%04X-%04X-%02X%02X-%02X%02X%02X%02X%02X%02X", - le32toh(guid->u.Uuid.time_low), le16toh(guid->u.Uuid.time_mid), - le16toh(guid->u.Uuid.time_high_and_version), - guid->u.Uuid.clock_seq_high_and_reserved, guid->u.Uuid.clock_seq_low, - guid->u.Uuid.node[0], guid->u.Uuid.node[1], guid->u.Uuid.node[2], - guid->u.Uuid.node[3], guid->u.Uuid.node[4], guid->u.Uuid.node[5]); -} - -/* Convert UTF16 string to UTF8. Rewritten from gpt utility. - * Caller must prepare enough space for UTF8. The rough estimation is: - * - * utf8 length = bytecount(utf16) * 1.5 +void GuidToStr(const Guid *guid, char *str, unsigned int buflen) { + require(buflen >= GUID_STRLEN); + require(snprintf(str, buflen, + "%08X-%04X-%04X-%02X%02X-%02X%02X%02X%02X%02X%02X", + le32toh(guid->u.Uuid.time_low), + le16toh(guid->u.Uuid.time_mid), + le16toh(guid->u.Uuid.time_high_and_version), + guid->u.Uuid.clock_seq_high_and_reserved, + guid->u.Uuid.clock_seq_low, + guid->u.Uuid.node[0], guid->u.Uuid.node[1], + guid->u.Uuid.node[2], guid->u.Uuid.node[3], + guid->u.Uuid.node[4], guid->u.Uuid.node[5]) == GUID_STRLEN-1); +} + +/* Convert possibly unterminated UTF16 string to UTF8. + * Caller must prepare enough space for UTF8, which could be up to + * twice the number of UTF16 chars plus the terminating '\0'. + * FIXME(wfrichar): The original implementation had security issues. As a + * temporary fix, I'm making this ONLY support ASCII codepoints. Bug 7542 + * (http://code.google.com/p/chromium-os/issues/detail?id=7542) is filed to fix + * this. */ -#define SIZEOF_GPTENTRY_NAME 36 /* sizeof(GptEntry.name[]) */ -void UTF16ToUTF8(const uint16_t *utf16, uint8_t *utf8) +void UTF16ToUTF8(const uint16_t *utf16, unsigned int maxinput, + uint8_t *utf8, unsigned int maxoutput) { - size_t s8idx, s16idx, s16len; + size_t s16idx, s8idx; uint32_t utfchar; - unsigned int next_utf16; - - for (s16len = 0; s16len < SIZEOF_GPTENTRY_NAME && utf16[s16len]; ++s16len); - - *utf8 = s8idx = s16idx = 0; - while (s16idx < s16len) { - utfchar = le16toh(utf16[s16idx++]); - if ((utfchar & 0xf800) == 0xd800) { - next_utf16 = le16toh(utf16[s16idx]); - if ((utfchar & 0x400) != 0 || (next_utf16 & 0xfc00) != 0xdc00) - utfchar = 0xfffd; - else - s16idx++; - } - if (utfchar < 0x80) { - utf8[s8idx++] = utfchar; - } else if (utfchar < 0x800) { - utf8[s8idx++] = 0xc0 | (utfchar >> 6); - utf8[s8idx++] = 0x80 | (utfchar & 0x3f); - } else if (utfchar < 0x10000) { - utf8[s8idx++] = 0xe0 | (utfchar >> 12); - utf8[s8idx++] = 0x80 | ((utfchar >> 6) & 0x3f); - utf8[s8idx++] = 0x80 | (utfchar & 0x3f); - } else if (utfchar < 0x200000) { - utf8[s8idx++] = 0xf0 | (utfchar >> 18); - utf8[s8idx++] = 0x80 | ((utfchar >> 12) & 0x3f); - utf8[s8idx++] = 0x80 | ((utfchar >> 6) & 0x3f); - utf8[s8idx++] = 0x80 | (utfchar & 0x3f); - } + + if (!utf16 || !maxinput || !utf8 || !maxoutput) + return; + + maxoutput--; /* plan for termination now */ + + for (s16idx = s8idx = 0; + s16idx < maxinput && utf16[s16idx] && maxoutput; + s16idx++, maxoutput--) { + utfchar = le16toh(utf16[s16idx]); + utf8[s8idx++] = utfchar & 0x7F; } utf8[s8idx++] = 0; } -/* Convert UTF8 string to UTF16. Rewritten from gpt utility. - * Caller must prepare enough space for UTF16. The conservative estimation is: - * - * utf16 bytecount = bytecount(utf8) / 3 * 4 +/* Convert UTF8 string to UTF16. The UTF8 string must be null-terminated. + * Caller must prepare enough space for UTF16, including a terminating 0x0000. + * FIXME(wfrichar): The original implementation had security issues. As a + * temporary fix, I'm making this ONLY support ASCII codepoints. Bug 7542 + * (http://code.google.com/p/chromium-os/issues/detail?id=7542) is filed to fix + * this. */ -void UTF8ToUTF16(const uint8_t *utf8, uint16_t *utf16) +void UTF8ToUTF16(const uint8_t *utf8, uint16_t *utf16, unsigned int maxoutput) { - size_t s16idx, s8idx, s8len; + size_t s16idx, s8idx; uint32_t utfchar; - unsigned int c, utfbytes; - - for (s8len = 0; utf8[s8len]; ++s8len); - - s8idx = s16idx = 0; - utfbytes = 0; - do { - c = utf8[s8idx++]; - if ((c & 0xc0) != 0x80) { - /* Initial characters. */ - if (utfbytes != 0) { - /* Incomplete encoding. */ - utf16[s16idx++] = 0xfffd; - } - if ((c & 0xf8) == 0xf0) { - utfchar = c & 0x07; - utfbytes = 3; - } else if ((c & 0xf0) == 0xe0) { - utfchar = c & 0x0f; - utfbytes = 2; - } else if ((c & 0xe0) == 0xc0) { - utfchar = c & 0x1f; - utfbytes = 1; - } else { - utfchar = c & 0x7f; - utfbytes = 0; - } - } else { - /* Followup characters. */ - if (utfbytes > 0) { - utfchar = (utfchar << 6) + (c & 0x3f); - utfbytes--; - } else if (utfbytes == 0) - utfbytes = -1; - utfchar = 0xfffd; - } - if (utfbytes == 0) { - if (utfchar >= 0x10000) { - utf16[s16idx++] = htole16(0xd800 | ((utfchar>>10)-0x40)); - if (s16idx >= SIZEOF_GPTENTRY_NAME) break; - utf16[s16idx++] = htole16(0xdc00 | (utfchar & 0x3ff)); - } else { - utf16[s16idx++] = htole16(utfchar); - } - } - } while (c != 0 && s16idx < SIZEOF_GPTENTRY_NAME); - if (s16idx < SIZEOF_GPTENTRY_NAME) - utf16[s16idx++] = 0; + + if (!utf8 || !utf16 || !maxoutput) + return; + + maxoutput--; /* plan for termination */ + + for (s8idx = s16idx = 0; + utf8[s8idx] && maxoutput; + s8idx++, maxoutput--) { + utfchar = utf8[s8idx]; + utf16[s16idx++] = utfchar & 0x7F; + } + utf16[s16idx++] = 0; } struct { @@ -499,14 +469,14 @@ static uint32_t GetSizeOfEntries(const GptData *gpt) { header = (GptHeader*)gpt->secondary_header; else return 0; - return header->number_of_entries; + return header->size_of_entry; } -GptEntry *GetEntry(GptData *gpt, int secondary, int entry_index) { +GptEntry *GetEntry(GptData *gpt, int secondary, uint32_t entry_index) { uint8_t *entries; - int stride = GetSizeOfEntries(gpt); - if (!stride) - return 0; + uint32_t stride = GetSizeOfEntries(gpt); + require(stride); + require(entry_index < GetNumberOfEntries(gpt)); if (secondary == PRIMARY) { entries = gpt->primary_entries; @@ -517,48 +487,48 @@ GptEntry *GetEntry(GptData *gpt, int secondary, int entry_index) { return (GptEntry*)(&entries[stride * entry_index]); } -void SetPriority(GptData *gpt, int secondary, int entry_index, int priority) { +void SetPriority(GptData *gpt, int secondary, uint32_t entry_index, + int priority) { GptEntry *entry; entry = GetEntry(gpt, secondary, entry_index); - - assert(priority >= 0 && priority <= CGPT_ATTRIBUTE_MAX_PRIORITY); + require(priority >= 0 && priority <= CGPT_ATTRIBUTE_MAX_PRIORITY); entry->attrs.fields.gpt_att &= ~CGPT_ATTRIBUTE_PRIORITY_MASK; entry->attrs.fields.gpt_att |= priority << CGPT_ATTRIBUTE_PRIORITY_OFFSET; } -int GetPriority(GptData *gpt, int secondary, int entry_index) { +int GetPriority(GptData *gpt, int secondary, uint32_t entry_index) { GptEntry *entry; entry = GetEntry(gpt, secondary, entry_index); return (entry->attrs.fields.gpt_att & CGPT_ATTRIBUTE_PRIORITY_MASK) >> CGPT_ATTRIBUTE_PRIORITY_OFFSET; } -void SetTries(GptData *gpt, int secondary, int entry_index, int tries) { +void SetTries(GptData *gpt, int secondary, uint32_t entry_index, int tries) { GptEntry *entry; entry = GetEntry(gpt, secondary, entry_index); - - assert(tries >= 0 && tries <= CGPT_ATTRIBUTE_MAX_TRIES); + require(tries >= 0 && tries <= CGPT_ATTRIBUTE_MAX_TRIES); entry->attrs.fields.gpt_att &= ~CGPT_ATTRIBUTE_TRIES_MASK; entry->attrs.fields.gpt_att |= tries << CGPT_ATTRIBUTE_TRIES_OFFSET; } -int GetTries(GptData *gpt, int secondary, int entry_index) { +int GetTries(GptData *gpt, int secondary, uint32_t entry_index) { GptEntry *entry; entry = GetEntry(gpt, secondary, entry_index); return (entry->attrs.fields.gpt_att & CGPT_ATTRIBUTE_TRIES_MASK) >> CGPT_ATTRIBUTE_TRIES_OFFSET; } -void SetSuccessful(GptData *gpt, int secondary, int entry_index, int success) { +void SetSuccessful(GptData *gpt, int secondary, uint32_t entry_index, + int success) { GptEntry *entry; entry = GetEntry(gpt, secondary, entry_index); - assert(success >= 0 && success <= CGPT_ATTRIBUTE_MAX_SUCCESSFUL); + require(success >= 0 && success <= CGPT_ATTRIBUTE_MAX_SUCCESSFUL); entry->attrs.fields.gpt_att &= ~CGPT_ATTRIBUTE_SUCCESSFUL_MASK; entry->attrs.fields.gpt_att |= success << CGPT_ATTRIBUTE_SUCCESSFUL_OFFSET; } -int GetSuccessful(GptData *gpt, int secondary, int entry_index) { +int GetSuccessful(GptData *gpt, int secondary, uint32_t entry_index) { GptEntry *entry; entry = GetEntry(gpt, secondary, entry_index); return (entry->attrs.fields.gpt_att & CGPT_ATTRIBUTE_SUCCESSFUL_MASK) >> @@ -714,13 +684,13 @@ int IsZero(const Guid *gp) { return (0 == memcmp(gp, &guid_unused, sizeof(Guid))); } -void PMBRToStr(struct pmbr *pmbr, char *str) { - char buf[256]; +void PMBRToStr(struct pmbr *pmbr, char *str, unsigned int buflen) { + char buf[GUID_STRLEN]; if (IsZero(&pmbr->boot_guid)) { - sprintf(str, "PMBR"); + require(snprintf(str, buflen, "PMBR") < buflen); } else { - GuidToStr(&pmbr->boot_guid, buf); - sprintf(str, "PMBR (Boot GUID: %s)", buf); + GuidToStr(&pmbr->boot_guid, buf, sizeof(buf)); + require(snprintf(str, buflen, "PMBR (Boot GUID: %s)", buf) < buflen); } } diff --git a/cgpt/cmd_add.c b/cgpt/cmd_add.c index 12ae57c9..21085e71 100644 --- a/cgpt/cmd_add.c +++ b/cgpt/cmd_add.c @@ -36,7 +36,7 @@ static void Usage(void) int cmd_add(int argc, char *argv[]) { struct drive drive; - int partition = 0; + uint32_t partition = 0; uint64_t begin = 0; uint64_t size = 0; Guid type_guid; @@ -57,8 +57,8 @@ int cmd_add(int argc, char *argv[]) { int gpt_retval; GptEntry *entry; - int index; - + uint32_t index; + int c; int errorcnt = 0; char *e = 0; @@ -198,7 +198,7 @@ int cmd_add(int argc, char *argv[]) { return CGPT_FAILED; } - int max_part = GetNumberOfEntries(&drive.gpt); + uint32_t max_part = GetNumberOfEntries(&drive.gpt); if (partition) { if (partition > max_part) { Error("invalid partition number: %d\n", partition); @@ -244,9 +244,8 @@ int cmd_add(int argc, char *argv[]) { if (set_unique) memcpy(&entry->unique, &unique_guid, sizeof(Guid)); if (label) { - uint16_t buf[128]; - UTF8ToUTF16((uint8_t *)label, buf); - memcpy(entry->name, buf, sizeof(entry->name)); + UTF8ToUTF16((uint8_t *)label, entry->name, + sizeof(entry->name) / sizeof(entry->name[0])); } if (set_raw) { entry->attrs.fields.gpt_att = raw_value; diff --git a/cgpt/cmd_boot.c b/cgpt/cmd_boot.c index 784684a2..56ab1d58 100644 --- a/cgpt/cmd_boot.c +++ b/cgpt/cmd_boot.c @@ -34,12 +34,12 @@ static void Usage(void) int cmd_boot(int argc, char *argv[]) { struct drive drive; - int partition = 0; + uint32_t partition = 0; char *bootfile = 0; int create_pmbr = 0; int retval = 1; int gpt_retval; - + int c; int errorcnt = 0; char *e = 0; @@ -63,7 +63,7 @@ int cmd_boot(int argc, char *argv[]) { case 'p': create_pmbr = 1; break; - + case 'h': Usage(); return CGPT_OK; @@ -98,7 +98,7 @@ int cmd_boot(int argc, char *argv[]) { Error("Unable to read PMBR\n"); goto done; } - + if (create_pmbr) { drive.pmbr.magic[0] = 0x1d; drive.pmbr.magic[1] = 0x9a; @@ -131,7 +131,7 @@ int cmd_boot(int argc, char *argv[]) { goto done; } - int index = partition - 1; + uint32_t index = partition - 1; GptEntry *entry = GetEntry(&drive.gpt, PRIMARY, index); memcpy(&drive.pmbr.boot_guid, &entry->unique, sizeof(Guid)); } @@ -152,9 +152,9 @@ int cmd_boot(int argc, char *argv[]) { close(fd); } - - char buf[256]; - GuidToStr(&drive.pmbr.boot_guid, buf); + + char buf[GUID_STRLEN]; + GuidToStr(&drive.pmbr.boot_guid, buf, sizeof(buf)); printf("%s\n", buf); // Write it all out diff --git a/cgpt/cmd_create.c b/cgpt/cmd_create.c index 16803709..6dabde3a 100644 --- a/cgpt/cmd_create.c +++ b/cgpt/cmd_create.c @@ -24,7 +24,7 @@ static void Usage(void) int cmd_create(int argc, char *argv[]) { struct drive drive; int zap = 0; - + int c; int errorcnt = 0; @@ -76,7 +76,7 @@ int cmd_create(int argc, char *argv[]) { drive.gpt.sector_bytes * GPT_ENTRIES_SECTORS); memset(drive.gpt.secondary_entries, 0, drive.gpt.sector_bytes * GPT_ENTRIES_SECTORS); - + drive.gpt.modified |= (GPT_MODIFIED_HEADER1 | GPT_MODIFIED_ENTRIES1 | GPT_MODIFIED_HEADER2 | GPT_MODIFIED_ENTRIES2); diff --git a/cgpt/cmd_find.c b/cgpt/cmd_find.c index 403d40f2..e8fe9b31 100644 --- a/cgpt/cmd_find.c +++ b/cgpt/cmd_find.c @@ -160,7 +160,7 @@ static int do_search(char *filename) { int i; struct drive drive; GptEntry *entry; - char partlabel[sizeof(entry->name) * 3 / 2]; + char partlabel[GPT_PARTNAME_LEN]; if (CGPT_OK != DriveOpen(filename, &drive)) return 0; @@ -181,7 +181,8 @@ static int do_search(char *filename) { (set_type && !memcmp(&type_guid, &entry->type, sizeof(Guid)))) { found = 1; } else if (set_label) { - UTF16ToUTF8(entry->name, (uint8_t *)partlabel); + UTF16ToUTF8(entry->name, sizeof(entry->name) / sizeof(entry->name[0]), + (uint8_t *)partlabel, sizeof(partlabel)); if (!strncmp(label, partlabel, sizeof(partlabel))) { found = 1; } @@ -250,7 +251,7 @@ static char *is_wholedev(const char *basename) { static int scan_real_devs(void) { int found = 0; char line[BUFSIZE]; - char partname[128]; + char partname[128]; // max size for /proc/partition lines? FILE *fp; char *pathname; @@ -264,7 +265,7 @@ static int scan_real_devs(void) { int ma, mi; long long unsigned int sz; - if (sscanf(line, " %d %d %llu %128[^\n ]", &ma, &mi, &sz, partname) != 4) + if (sscanf(line, " %d %d %llu %127[^\n ]", &ma, &mi, &sz, partname) != 4) continue; if ((pathname = is_wholedev(partname))) { diff --git a/cgpt/cmd_show.c b/cgpt/cmd_show.c index 6ab537dc..a9963b5b 100644 --- a/cgpt/cmd_show.c +++ b/cgpt/cmd_show.c @@ -6,7 +6,7 @@ #define __STDC_FORMAT_MACROS #include <getopt.h> -#include <inttypes.h> +#include <inttypes.h> #include <stdio.h> #include <stdlib.h> #include <string.h> @@ -80,7 +80,7 @@ static void HeaderDetails(GptHeader *header, const char *indent, int raw) { printf("%sSig: ", indent); if (!raw) { - printf("["); + printf("["); for (i = 0; i < sizeof(header->signature); ++i) printf("%c", header->signature[i]); printf("]"); @@ -101,7 +101,7 @@ static void HeaderDetails(GptHeader *header, const char *indent, int raw) { { /* For disk guid */ char buf[GUID_STRLEN]; - GuidToStr(&header->disk_uuid, buf); + GuidToStr(&header->disk_uuid, buf, GUID_STRLEN); printf("%sDisk UUID: %s\n", indent, buf); } @@ -111,25 +111,27 @@ static void HeaderDetails(GptHeader *header, const char *indent, int raw) { printf("%sEntries CRC: 0x%08x\n", indent, header->entries_crc32); } -void EntryDetails(GptEntry *entry, int index, int raw) { - char contents[256]; - uint8_t label[sizeof(entry->name) * 3 / 2]; +void EntryDetails(GptEntry *entry, uint32_t index, int raw) { + char contents[256]; // scratch buffer for formatting output + uint8_t label[GPT_PARTNAME_LEN]; if (!raw) { - char type[GUID_STRLEN], unique[GUID_STRLEN];; + char type[GUID_STRLEN], unique[GUID_STRLEN]; - UTF16ToUTF8(entry->name, label); - snprintf(contents, sizeof(contents), "Label: \"%s\"", label); + UTF16ToUTF8(entry->name, sizeof(entry->name) / sizeof(entry->name[0]), + label, sizeof(label)); + require(snprintf(contents, sizeof(contents), + "Label: \"%s\"", label) < sizeof(contents)); printf(PARTITION_FMT, (int)entry->starting_lba, (int)(entry->ending_lba - entry->starting_lba + 1), index+1, contents); if (CGPT_OK == ResolveType(&entry->type, type)) { printf(PARTITION_MORE, "Type: ", type); } else { - GuidToStr(&entry->type, type); + GuidToStr(&entry->type, type, GUID_STRLEN); printf(PARTITION_MORE, "Type: ", type); } - GuidToStr(&entry->unique, unique); + GuidToStr(&entry->unique, unique, GUID_STRLEN); printf(PARTITION_MORE, "UUID: ", unique); if (!memcmp(&guid_chromeos_kernel, &entry->type, sizeof(Guid))) { int tries = (entry->attrs.fields.gpt_att & @@ -141,31 +143,34 @@ void EntryDetails(GptEntry *entry, int index, int raw) { int priority = (entry->attrs.fields.gpt_att & CGPT_ATTRIBUTE_PRIORITY_MASK) >> CGPT_ATTRIBUTE_PRIORITY_OFFSET; - snprintf(contents, sizeof(contents), - "priority=%d tries=%d successful=%d", - priority, tries, successful); + require(snprintf(contents, sizeof(contents), + "priority=%d tries=%d successful=%d", + priority, tries, successful) < sizeof(contents)); printf(PARTITION_MORE, "Attr: ", contents); } } else { char type[GUID_STRLEN], unique[GUID_STRLEN]; - UTF16ToUTF8(entry->name, label); - snprintf(contents, sizeof(contents), "Label: \"%s\"", label); + UTF16ToUTF8(entry->name, sizeof(entry->name) / sizeof(entry->name[0]), + label, sizeof(label)); + require(snprintf(contents, sizeof(contents), + "Label: \"%s\"", label) < sizeof(contents)); printf(PARTITION_FMT, (int)entry->starting_lba, (int)(entry->ending_lba - entry->starting_lba + 1), index+1, contents); - GuidToStr(&entry->type, type); + GuidToStr(&entry->type, type, GUID_STRLEN); printf(PARTITION_MORE, "Type: ", type); - GuidToStr(&entry->unique, unique); + GuidToStr(&entry->unique, unique, GUID_STRLEN); printf(PARTITION_MORE, "UUID: ", unique); - snprintf(contents, sizeof(contents), "[%x]", entry->attrs.fields.gpt_att); + require(snprintf(contents, sizeof(contents), + "[%x]", entry->attrs.fields.gpt_att) < sizeof(contents)); printf(PARTITION_MORE, "Attr: ", contents); } } void EntriesDetails(GptData *gpt, const int secondary, int raw) { - int i; + uint32_t i; for (i = 0; i < GetNumberOfEntries(gpt); ++i) { GptEntry *entry; @@ -182,7 +187,7 @@ int cmd_show(int argc, char *argv[]) { int numeric = 0; int verbose = 0; int quick = 0; - int partition = 0; + uint32_t partition = 0; int single_item = 0; int gpt_retval; @@ -268,9 +273,9 @@ int cmd_show(int argc, char *argv[]) { return CGPT_FAILED; } - int index = partition - 1; + uint32_t index = partition - 1; GptEntry *entry = GetEntry(&drive.gpt, PRIMARY, index); - char buf[256]; + char buf[256]; // scratch buffer for string conversion if (single_item) { switch(single_item) { @@ -281,15 +286,16 @@ int cmd_show(int argc, char *argv[]) { printf("%" PRId64 "\n", entry->ending_lba - entry->starting_lba + 1); break; case 't': - GuidToStr(&entry->type, buf); + GuidToStr(&entry->type, buf, sizeof(buf)); printf("%s\n", buf); break; case 'u': - GuidToStr(&entry->unique, buf); + GuidToStr(&entry->unique, buf, sizeof(buf)); printf("%s\n", buf); break; case 'l': - UTF16ToUTF8(entry->name, (uint8_t *)buf); + UTF16ToUTF8(entry->name, sizeof(entry->name) / sizeof(entry->name[0]), + (uint8_t *)buf, sizeof(buf)); printf("%s\n", buf); break; case 'S': @@ -311,7 +317,7 @@ int cmd_show(int argc, char *argv[]) { } } else if (quick) { // show all partitions, quickly - int i; + uint32_t i; GptEntry *entry; char type[GUID_STRLEN]; @@ -323,7 +329,7 @@ int cmd_show(int argc, char *argv[]) { if (!numeric && CGPT_OK == ResolveType(&entry->type, type)) { } else { - GuidToStr(&entry->type, type); + GuidToStr(&entry->type, type, GUID_STRLEN); } printf(PARTITION_FMT, (int)entry->starting_lba, (int)(entry->ending_lba - entry->starting_lba + 1), @@ -338,8 +344,8 @@ int cmd_show(int argc, char *argv[]) { } printf(TITLE_FMT, "start", "size", "part", "contents"); - char buf[256]; - PMBRToStr(&drive.pmbr, buf); + char buf[256]; // buffer for formatted PMBR content + PMBRToStr(&drive.pmbr, buf, sizeof(buf)); // will exit if buf is too small printf(GPT_FMT, 0, GPT_PMBR_SECTOR, "", buf); if (drive.gpt.valid_headers & MASK_PRIMARY) { @@ -349,7 +355,7 @@ int cmd_show(int argc, char *argv[]) { GptHeader *header; char indent[64]; - snprintf(indent, sizeof(indent), GPT_MORE); + require(snprintf(indent, sizeof(indent), GPT_MORE) < sizeof(indent)); header = (GptHeader*)drive.gpt.primary_header; HeaderDetails(header, indent, numeric); } @@ -400,7 +406,7 @@ int cmd_show(int argc, char *argv[]) { GptHeader *header; char indent[64]; - snprintf(indent, sizeof(indent), GPT_MORE); + require(snprintf(indent, sizeof(indent), GPT_MORE) < sizeof(indent)); header = (GptHeader*)drive.gpt.secondary_header; HeaderDetails(header, indent, numeric); } @@ -412,6 +418,3 @@ int cmd_show(int argc, char *argv[]) { return CGPT_OK; } - - - diff --git a/utility/vbutil_keyblock.c b/utility/vbutil_keyblock.c index dd13eb3d..ff1b44be 100644 --- a/utility/vbutil_keyblock.c +++ b/utility/vbutil_keyblock.c @@ -127,7 +127,12 @@ static int Unpack(const char* infile, const char* datapubkey, /* If the block is signed, then verify it with the signing public key, since KeyBlockRead() only verified the hash. */ - if (block->key_block_signature.sig_size && signpubkey) { + if (block->key_block_signature.sig_size) { + if (!signpubkey) { + fprintf(stderr, + "vbutil_keyblock: keyblock requires public key to verify\n"); + return 1; + } sign_key = PublicKeyRead(signpubkey); if (!sign_key) { fprintf(stderr, "vbutil_keyblock: Error reading signpubkey.\n"); @@ -137,12 +142,16 @@ static int Unpack(const char* infile, const char* datapubkey, fprintf(stderr, "vbutil_keyblock: Error verifying key block.\n"); return 1; } + printf("Signature algorithm: %" PRIu64 " %s\n", sign_key->algorithm, + (sign_key->algorithm < kNumAlgorithms ? + algo_strings[sign_key->algorithm] : "(invalid)")); Free(sign_key); + } else { + printf("Signature Algorithm: <none>\n"); } printf("Key block file: %s\n", infile); - printf("Signature %s\n", sign_key ? "valid" : "ignored"); - printf("Flags: %" PRIu64 " ", block->key_block_flags); + printf("Flags: %" PRIu64 "\n", block->key_block_flags); if (block->key_block_flags & KEY_BLOCK_FLAG_DEVELOPER_0) printf(" !DEV"); if (block->key_block_flags & KEY_BLOCK_FLAG_DEVELOPER_1) |