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

7 files changed, 198 insertions, 212 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;
 }
-
-
-