Since `strcpy`, `strcat`, and `sprintf` are dangerous, what shall we use in stead of them?

Question

In Computer Systems: a Programmer's Perspective,

Unfortunately, a number of commonly used library functions, including strcpy, strcat, and sprintf, have the property that they can generate a byte sequence without being given any indication of the size of the destination buffer [97]. Such conditions can lead to vulnerabilities to buffer overflow.

Since strcpy, strcat, and sprintf are dangerous, what shall we use instead of them, or how shall we use them correctly/securely?

---

In the book, there is an example of implementing a simple shell, which calls strcpy(). I was wondering what purpose is strcpy(buf, cmdline) in eval()? Does it introduce security vulnerability, or does it improve security, by limiting the size of the char array to an known size? Is it a regular way for security coding?

What if we don't know the max limit of a command line length?

Thanks.

#include "csapp.h"
#define MAXARGS 128

/* Function prototypes */
void eval(char *cmdline);
int parseline(char *buf, char **argv);
int builtin_command(char **argv);

int main()
{
  char cmdline[MAXLINE]; /* Command line */

  while (1) {
    /* Read */
    printf("> ");
    Fgets(cmdline, MAXLINE, stdin);
    if (feof(stdin))
      exit(0);

    /* Evaluate */
    eval(cmdline);
  }
}


/* eval - Evaluate a command line */
void eval(char *cmdline)
{
  char *argv[MAXARGS]; /* Argument list execve() */
  char buf[MAXLINE]; /* Holds modified command line */
  int bg; /* Should the job run in bg or fg? */
  pid_t pid; /* Process id */

  strcpy(buf, cmdline);
  bg = parseline(buf, argv);
  if (argv[0] == NULL)
    return; /* Ignore empty lines */

  if (!builtin_command(argv)) {
    if ((pid = Fork()) == 0) { /* Child runs user job */
      if (execve(argv[0], argv, environ) < 0) {
    printf("%s: Command not found.\n", argv[0]);
    exit(0);
      }
    }

    /* Parent waits for foreground job to terminate */
    if (!bg) {
      int status;
      if (waitpid(pid, &status, 0) < 0)
    unix_error("waitfg: waitpid error");
    }
    else
      printf("%d %s", pid, cmdline);
  }
  return;
}

/* If first arg is a builtin command, run it and return true */
int builtin_command(char **argv)
{
  if (!strcmp(argv[0], "quit")) /* quit command */
    exit(0);
  if (!strcmp(argv[0], "&")) /* Ignore singleton & */
    return 1;
  return 0; /* Not a builtin command */
}


/* parseline - Parse the command line and build the argv array */
int parseline(char *buf, char **argv)
{
  char *delim; /* Points to first space delimiter */
  int argc; /* Number of args */
  int bg; /* Background job? */

  buf[strlen(buf)-1] = ’ ’; /* Replace trailing ’\n’ with space */
  while (*buf && (*buf == ’ ’)) /* Ignore leading spaces */
    buf++;

  /* Build the argv list */
  argc = 0;
  while ((delim = strchr(buf, ’ ’))) {
    argv[argc++] = buf;
    *delim = ’\0’;
    buf = delim + 1;
    while (*buf && (*buf == ’ ’)) /* Ignore spaces */
      buf++;
  }
  argv[argc] = NULL;

  if (argc == 0) /* Ignore blank line */
    return 1;

  /* Should the job run in the background? */
  if ((bg = (*argv[argc-1] == ’&’)) != 0)
    argv[--argc] = NULL;

  return bg;
}

Answer 1

The strncpy() strncat(), and snprintf() functions include the output buffer length as a parameter in order to prevent overflow. They still have a problem with the terminal null that can be resolved by storing a null at then end of the buffer.

Answer 2

Part of my answer was mentioned in the already given answers, but I am trying to give a more complete picture, so please excuse the repetitions.

The problem with those functions is that, in case one wants them to be

• safe, and
• working correctly for (almost) any possible input, with no assumptions about the input data size, and never loosing any of the input characters

then one cannot simply use a constant size character buffer as target any more - one has to use a dynamically allocated buffer, which forbids usage of simple, locally declared buffer variables.

Unfortunately, in the C standard libraries, AFAIK there are no standardized, portable alternatives which work on dynamically sized targets. This leaves you with the following alternatives:

• implement your own versions of such functions (as suggested in gnasher's answer)

• switch to C++ and use something like std::string (also suggested in gnasher's answer)

• don't write portable code and use platform-dependend functions like the Linux-specific asprintf (as mentioned in Deduplicator`s comment below gnashers's answer)

• use a portable version of asprintf from Stackoverflow

• make assumptions about the required maximum size of the target buffer and use portable functions like strncpy (as mentioned in user230118's answer) to prevent buffer-overflows

• make the same assumptions, and use non-portable functions like the Windows-specific strcpy_s

• write your own, portable version of strcpy_s, maybe better suited for your specific use case

As you see, it is a tradeoff, there is no single "best" solution here.

Answer 3

Back in the day when I used C a lot, I wrote a function malloc_printf which created a block of the right size and wrote into that, returning the block to the caller to free it. strcat and strcpy are trivial to implement with this. It doesn’t only protect you from buffer overflows, it also protects you from buffers that are too small.

Nowadays I use a C++ std::string or an Objective-C NSString or a Swift String and have none of these problems.

Answer 4

Since strcpy, strcat, and sprintf are dangerous, ...

strcpy, strcat, and perhaps sprintf are better used as building blocks for safer functions than used as safe functions unto themselves.

If due to the local usage of strcpy, strcat, overrun is not possible, then there is is no safety concern (aside from possible buffer overlap).

---

How to form a safer function?

We could attempt to form our own plodding "safe" versions:

char *strcpy_safe1(size_t *dest_sz, char *dest, const char *source) {
  size_t src_sz = strlen(source) + 1;
  if (src_sz <= dest_sz) {
    return strcpy(dest, src);
  }
  Handle_errors();
}

char *strcat_safe1(size_t *dest_sz, char *dest, const char *source) {
  size_t dest_len = strlen(dest);
  if (dest_len < dest_sz) {
     strcpy_safe1(dest_sz - dest_len, dest + dest_len, source);
     return dest;
  }
  Handle_errors();
}

Notice code is incomplete on error handling - leaving only stub code.

To make these string handling functions safe we need to assess performance cost, error detection, and error handing. Since what is most important various among cases, the best answer varies.

---

what shall we use in-stead of them?

We need an error model.

• Exit on error: Pretty harsh, yet easy to code.

• Return NULL: Calling code is now saddled with checking each and every function result. Yet easy to write.

• Global Error flag: Use a errno or errno like system. In my experience, errno is not so popular.

• Return truncated strings: Hard to tell if a problem occurred.

• Return a string and error flag: A little better than the errno, yet still obliges lots of checking.

• Call a global error handier Something like the _s functions. Interesting idea, but has not caught on. Common implementations have short falls.

The lists goes on and on. The best answer is situation dependent and is part of the reason C has not formed great well received alternatives: it depends on lot on how one wants to handle errors and how much performance impact is tolerable.

---

snprintf() is a good alterative to sprintf().

To test if a failure occurred:

 int len = snprintf(s, s_size, ..........
 if (len < 0 || (unsigned) len >= s_size) {
   Error();
 }

---

What if we don't know the max limit of a command line length?

The max limit is known, somewhere, maybe many function levels up in the calling tree. To make safe code, this size info is needed to be conveyed down to the lower functions - else you are SOL.