On the Difference Between strlcat and strncat

16 Jul 2019

Author: Sviatoslav Razmyslov

Description of the Functions
Bug in Haiku OS
P.S.

While we are working hard on writing big articles on code check of the Haiku operating system, I'd like to give an example of an often found error with strncat function taken from that project. It might be useful for all the C and C++ developers to refresh their knowledge on this topic.

Description of the Functions

The strncat function is used for string concatenation and has the following signature:

char *strncat(char *dest, const char *src, size_t n);

It adds not more than n symbols from the src string to the dest string however, the src string may not end with terminal null. There should be enough space in the dest string, otherwise, program behavior becomes unpredictable because of the buffer overflow for the function does not produce borders control.

The strlcat function is used for string concatenation and, compared to strncat function, it's safer to use one. The strlcat function has the following signature:

size_t strlcat(char *dst, const char *src, size_t size)

Unlike the other functions, it takes the whole buffer size and guarantees the presence of terminal symbol at the result. For the strlcat function proper operation, you need to transmit only null-terminated strings.

Bug in Haiku OS

V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 101

static void
dump_acpi_namespace(acpi_ns_device_info *device, char *root, int indenting)
{
  char result[255];
  char output[320];
  char tabs[255] = "";
  char hid[16] = "";
  int i;
  size_t written = 0;
  for (i = 0; i < indenting; i++)
    strlcat(tabs, "|    ", sizeof(tabs));

  strlcat(tabs, "|--- ", sizeof(tabs));
  ....
  void *counter = NULL;
  while (....) {
    uint32 type = device->acpi->get_object_type(result);
    snprintf(output, sizeof(output), "%s%s", tabs, result + depth);
    switch(type) {
      case ACPI_TYPE_INTEGER:
        strncat(output, "     INTEGER", sizeof(output));
        break;
      case ACPI_TYPE_STRING:
        strncat(output, "     STRING", sizeof(output));
        break;
      case ACPI_TYPE_BUFFER:
        strncat(output, "     BUFFER", sizeof(output));
        break;
      case ACPI_TYPE_PACKAGE:
        strncat(output, "     PACKAGE", sizeof(output));
        break;
      ....
      case ACPI_TYPE_MUTEX:
        strncat(output, "     MUTEX", sizeof(output));
        break;
      case ACPI_TYPE_REGION:
        strncat(output, "     REGION", sizeof(output));
        break;
      case ACPI_TYPE_POWER:
        strncat(output, "     POWER", sizeof(output));
        break;
      case ACPI_TYPE_PROCESSOR:
        strncat(output, "     PROCESSOR", sizeof(output));
        break;
      case ACPI_TYPE_THERMAL:
        strncat(output, "     THERMAL", sizeof(output));
        break;
      case ACPI_TYPE_BUFFER_FIELD:
        strncat(output, "     BUFFER_FIELD", sizeof(output));
        break;
      case ACPI_TYPE_ANY:
      default:
        break;
    }
    ....
  }
  ....
}

The analyzer detected the mixed code consisting of strlcat and strncat functions calls. However, the strlcat function calls are correct:

char tabs[255] = "";
....
strlcat(tabs, "|--- ", sizeof(tabs));

they transmit null-terminated string and the whole buffer size.

At the same time, multiple strncat calls in a loop are false and may lead to an error:

char output[320];
....
strncat(output, "     INTEGER", sizeof(output));

A program may operate sustainably for a long time if short strings entry the function but the buffer limit may be exceeded quickly in the loop.

P.S.

We have already sent the report to the Haiku OS developers without waiting for the major big articles to be published, and they have already begun fixing the bugs: https://git.haiku-os.org/haiku/log/?qt=grep&q=pvs

#Cpp #Knowledge