-4

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;
}
5
  • what does parseline() do? The comment for buf says it holds the modified command line. Is this perhaps related to variable expansion?
    – amon
    Oct 24, 2020 at 12:47
  • parseline - Parse the command line and build the argv array. No variable expansion is implemented.
    – Tim
    Oct 24, 2020 at 12:55
  • 2
    Cross-posted on Code Review
    – Mast
    Oct 24, 2020 at 13:16
  • @Tim I just ignore the advice (and warnings on MSVC with _CRT_SECURE_NO_WARNINGS). There aren't cross-platform variants to these functions in C. Unless you want to take the time to write your own that protect against buffer overruns and the like, I figure I'm using C. I'm playing with razors anyway. Give me a straight razor for shaving (just like in real life -- I love straight razors). That's C. I'm not using a safety razor here. That's the other languages. If security is a huge concern, then I either switch to a diff language or write my own functions for strcpy, sprintf, and the like.
    – user377672
    Oct 24, 2020 at 16:11
  • This is actually quite a practical question from someone concerned with safety, isn't it? We don't have cross-platform alternatives as of yet like MS's sprintf_s to replace our code. It's kind of leaving us to the gutters if people just say we shouldn't use something without suggesting an alternative we can actually use without reducing our demographics to just Windows users. Well, I don't really understand the voting patterns on this site. Makes zero sense to me.
    – user377672
    Oct 26, 2020 at 17:19

4 Answers 4

5

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.

8
  • strncpy etc. are totally unusable if you use UTF-8, which is nowadays most common, because it can cut off in the middle of a code point, leaving you with an invalid string. The much bigger problem is that it may prevent a buffer overflow, but you don't get a copy of the input string, or a concatenation of the input strings.
    – gnasher729
    Oct 24, 2020 at 21:47
  • 2
    @gnasher729 If your text is longer than the buffer length you've calculated, it's better to have a corrupted UTF-8 code point than a buffer overflow!
    – IMSoP
    Oct 24, 2020 at 21:57
  • 1
    One of those is not like the others: It doesn't actually work on nul-terminated strings. Not knowing that will lead to interesting effects. Oct 24, 2020 at 23:37
  • The original post was worried about destination buffer overflows which the strN functions handle, except for the string termination problem.
    – cwallach
    Oct 25, 2020 at 4:25
  • Do you actually know that strncpy() is the only one of them which does not leave a nul-terminated string if the input is long enough, and it always writes the whole buffer, as it is the only one that is not a string-function with a length? If you do, warn about it, there are already far too many who don't. Oct 25, 2020 at 8:55
2

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.

1
  • asprintf() looks like your custom one. And sure, adapting vsnprintf() to allow printing to a std::string or NSString isn't that hard. Oct 24, 2020 at 14:27
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.

2
  • Wish this place respected late answers more. Nicely done. Oct 25, 2020 at 13:20
  • What I wrote was very similar to the “portable” version except using completely portable code, which is no problem.
    – gnasher729
    Oct 25, 2020 at 13:53
1

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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.