Audit possible string buffer overruns.

* runtime/transport/procfs.c (_stp_register_ctl_channel_fs): Change
  sprintf() call to snprintf() call.
  (_stp_unregister_ctl_channel_fs): Ditto.
* runtime/transport/ring_buffer.c (_stp_transport_data_fs_init): Ditto.
* runtime/unwind.c (_stp_eh_enc_name): Ditto.
* runtime/io.c: Make sure STP_LOG_BUF_LEN isn't too short.
* runtime/staprun/staprun_funcs.c: Add comments about strcpy/strcat/sprintf
  appropriateness.
  (check_signature): Fix string length calculation.
* runtime/staprun/common.c: Add comments about strcpy/strcat/sprintf
  appropriateness.
* runtime/staprun/modverify.c: Ditto.
* runtime/staprun/relay.c: Ditto.
* runtime/staprun/relay_old.c: Ditto.
* runtime/staprun/staprun_funcs.c: Ditto.

diff --git a/runtime/io.c b/runtime/io.c
index 0136aae568396bfe4386e96720752b7b4ba72433..b133620e2cedeea6ef827ea34d923ed813aeb604 100644 (file)
--- a/runtime/io.c
+++ b/runtime/io.c
@@ -20,6 +20,10 @@
 
 #define WARN_STRING "WARNING: "
 #define ERR_STRING "ERROR: "
+#if (STP_LOG_BUF_LEN < 10) /* sizeof(WARN_STRING) */
+#error "STP_LOG_BUF_LEN is too short"
+#endif
+
 enum code { INFO=0, WARN, ERROR, DBUG };
 
 static void _stp_vlog (enum code type, const char *func, int line, const char *fmt, va_list args)
@@ -34,9 +38,13 @@ static void _stp_vlog (enum code type, const char *func, int line, const char *f
        if (type == DBUG) {
                start = _stp_snprintf(buf, STP_LOG_BUF_LEN, "%s:%d: ", func, line);
        } else if (type == WARN) {
+               /* This strcpy() is OK, since we know STP_LOG_BUF_LEN
+                * is > sizeof(WARN_STRING). */
                strcpy (buf, WARN_STRING);
                start = sizeof(WARN_STRING) - 1;
        } else if (type == ERROR) {
+               /* This strcpy() is OK, since we know STP_LOG_BUF_LEN
+                * is > sizeof(ERR_STRING) (which is < sizeof(WARN_STRING). */
                strcpy (buf, ERR_STRING);
                start = sizeof(ERR_STRING) - 1;
        }

diff --git a/runtime/staprun/common.c b/runtime/staprun/common.c
index f5fbf2286c923f4137a42220e7ad157c2be6b412..4fbc9e5c463d22e3d0f75bea3368453035386779 100644 (file)
--- a/runtime/staprun/common.c
+++ b/runtime/staprun/common.c
@@ -52,6 +52,8 @@ static char *get_abspath(char *path)
        if (len + 2 + strlen(path) >= PATH_MAX)
                return NULL;
        path_buf[len] = '/';
+       /* Note that this strcpy() call is OK, since we checked
+        * the length earlier to make sure the string would fit. */
        strcpy(&path_buf[len + 1], path);
        return path_buf;
 }
@@ -81,6 +83,9 @@ int make_outfile_name(char *buf, int max, int fnum, int cpu, time_t t, int bulk)
        }
        /* special case: for testing we sometimes want to write to /dev/null */
        if (strcmp(outfile_name, "/dev/null") == 0) {
+               /* This strcpy() call is OK since we know that the
+                * buffer is at least PATH_MAX bytes long at this
+                * point. */
                strcpy(buf, "/dev/null");
        } else {
                if (bulk) {

diff --git a/runtime/staprun/modverify.c b/runtime/staprun/modverify.c
index 3fa3047b7f7d84564c85ac9c331c2c83a67e9872..402371f34df91183df0c3fb11048b03524a3c528 100644 (file)
--- a/runtime/staprun/modverify.c
+++ b/runtime/staprun/modverify.c
@@ -188,6 +188,8 @@ check_cert_db_permissions (const char *cert_db_path) {
       return 0;
     }
 
+  /* These uses of sprintf() are OK, since we just allocated the
+   * string to be the correct length. */
   sprintf (fileName, "%s/cert8.db", cert_db_path);
   rc &= check_db_file_permissions (fileName);
   sprintf (fileName, "%s/key3.db", cert_db_path);

diff --git a/runtime/staprun/relay.c b/runtime/staprun/relay.c
index 28fba651d8348117db2de4435bd99bb5b21b20e7..ec4ba28a84a21d2c0be151eea398eba8093d2c27 100644 (file)
--- a/runtime/staprun/relay.c
+++ b/runtime/staprun/relay.c
@@ -295,6 +295,9 @@ int init_relayfs(void)
                        if (outfile_name) {
                                /* special case: for testing we sometimes want to write to /dev/null */
                                if (strcmp(outfile_name, "/dev/null") == 0) {
+                                       /* This strcpy() is OK, since
+                                        * we know buf is PATH_MAX
+                                        * bytes long. */
                                        strcpy(buf, "/dev/null");
                                } else {
                                        len = stap_strfloctime(buf, PATH_MAX,

diff --git a/runtime/staprun/relay_old.c b/runtime/staprun/relay_old.c
index 8dfcc16b426885415f8308a3f97e51b6f0016730..85cff6dfedf1c07bec686e102379d041597fe0bf 100644 (file)
--- a/runtime/staprun/relay_old.c
+++ b/runtime/staprun/relay_old.c
@@ -163,6 +163,8 @@ static int open_relayfs_files(int cpu, const char *relay_filebase, const char *p
                /* special case: for testing we sometimes want to
                 * write to /dev/null */
                if (strcmp(outfile_name, "/dev/null") == 0) {
+                       /* This strcpy() is OK, since we know tmp is
+                        * PATH_MAX bytes long. */
                        strcpy(tmp, "/dev/null");
                } else {
                        int len;

diff --git a/runtime/staprun/staprun_funcs.c b/runtime/staprun/staprun_funcs.c
index 9771986b86158016c624aa02366883d7af575b3f..df464d0a1ce50ac8c8b1a151a3883d1ccec049ee 100644 (file)
--- a/runtime/staprun/staprun_funcs.c
+++ b/runtime/staprun/staprun_funcs.c
@@ -73,6 +73,8 @@ int insert_module(
                        _perr("[re]allocating memory failed");
                        return -1;
                }
+               /* Note that these strcat() calls are OK, since we just
+                * allocated space for the resulting string. */
                strcat(opts, " ");
                strcat(opts, options[i]);
        }
@@ -247,10 +249,12 @@ check_signature(const char *path, const void *module_data, off_t module_size)
 
   /* Add the .sgn suffix to the canonicalized module path to get the signature
      file path.  */
-  if (strlen (path) >= PATH_MAX - 4) {
+  if (strlen (path) >= PATH_MAX - 5) {
     err("Path \"%s.sgn\" is too long.", path);
     return -1;
   }
+  /* This use of sprintf() is OK, since we just checked the final
+   * string's length. */
   sprintf (signature_realpath, "%s.sgn", path);
 
   rc = verify_module (signature_realpath, path, module_data, module_size);
@@ -301,6 +305,8 @@ check_stap_module_path(const char *module_path, int module_fd)
         * /lib/modules/`uname -r`/systemtapmod.ko, put a '/' on the
         * end of staplib_dir_realpath. */
        if (strlen(staplib_dir_realpath) < (PATH_MAX - 1))
+               /* Note that this strcat() is OK, since we just
+                * checked the length of the resulting string.  */
                strcat(staplib_dir_realpath, "/");
        else {
                err("ERROR: Path \"%s\" is too long.", staplib_dir_realpath);

diff --git a/runtime/transport/procfs.c b/runtime/transport/procfs.c
index 9e05cc147a8c531d7e7a5b704d1376e01e38dfbd..4910173187beeb9d102a7347c8b2980faa1941b0 100644 (file)
--- a/runtime/transport/procfs.c
+++ b/runtime/transport/procfs.c
@@ -110,7 +110,7 @@ static int _stp_register_ctl_channel_fs(void)
 #ifdef STP_BULKMODE
        /* now for each cpu "n", create /proc/systemtap/module_name/n  */
        stp_for_each_cpu(i) {
-               sprintf(buf, "%d", i);
+               snprintf(buf, sizeof(buf), "%d", i);
                de = create_proc_entry(buf, 0600, _stp_proc_root);
                if (de == NULL)
                        goto err1;
@@ -144,7 +144,7 @@ err1:
        stp_for_each_cpu(j) {
                if (j == i)
                        break;
-               sprintf(buf, "%d", j);
+               snprintf(buf, sizeof(buf), "%d", j);
                remove_proc_entry(buf, _stp_proc_root);
 
        }
@@ -168,7 +168,7 @@ static void _stp_unregister_ctl_channel_fs(void)
                _stp_kfree(de->data);
 
        stp_for_each_cpu(i) {
-               sprintf(buf, "%d", i);
+               snprintf(buf, sizeof(buf), "%d", i);
                remove_proc_entry(buf, _stp_proc_root);
        }
        remove_proc_entry("bufsize", _stp_proc_root);

diff --git a/runtime/transport/ring_buffer.c b/runtime/transport/ring_buffer.c
index 232f09993c650e5c212a342775bfe05c0d297e5b..f0d195cb712f9273544b96992e5cbeb402760703 100644 (file)
--- a/runtime/transport/ring_buffer.c
+++ b/runtime/transport/ring_buffer.c
@@ -696,7 +696,7 @@ static int _stp_transport_data_fs_init(void)
                        _stp_transport_data_fs_close();
                        return -EINVAL;
                }
-               sprintf(cpu_file, "trace%d", cpu);
+               snprintf(cpu_file, sizeof(cpu_file), "trace%d", cpu);
                __stp_entry[cpu] = debugfs_create_file(cpu_file, 0600,
                                                       _stp_get_module_dir(),
                                                       (void *)(long)cpu,

diff --git a/runtime/unwind.c b/runtime/unwind.c
index da39223a8b0040696f3b6a6d189a73a6565c0ee9..3e569657c304555402f425056cee5a64c9fcbc9d 100644 (file)
--- a/runtime/unwind.c
+++ b/runtime/unwind.c
@@ -532,7 +532,7 @@ static char *_stp_eh_enc_name(signed type)
        hi = (type & DW_EH_PE_ADJUST) >> 4;
        low = type & DW_EH_PE_FORM;
        if (hi > 5 || low > 4 || (low == 0 && (type & DW_EH_PE_signed))) {
-               sprintf(buf, "ERROR:encoding=0x%x", type);
+           snprintf(buf, sizeof(buf), "ERROR:encoding=0x%x", type);
                return buf;
        }