How do I prevent string overflow in C?

Question

I'm trying to make a program which concatenates two strings. The max length of the strings should be 50 characters and I'm making a string with that size. I'm getting the strings using argv. How can I detect if the strings are over 50 characters? Can I do it without playing around with memory since I haven't learned this yet. The function for concatenation is stored in a mystrings.h file Here's my current code:

#include <stdio.h>
#include <string.h>
#include "mystrings.h"

int main(int argc, char *argv[]) {
    if (argc == 3) {
        char str1[50];
        char str2[50];

        strcpy(str1, argv[1]);
        strcpy(str2, argv[2]);

        strConcat(str1, str2);

        printf("Concatenated string: %s\n", str1);
    } else {
        printf("Invalid number of arguments passed. Format required:\n <STRING1> <STRING2>\n");
    }
}

Have you seen [Safe way to concat two strings in C](https://stackoverflow.com/q/36437461/1115360)? — Andrew Morton, Nov 24 '22 at 18:29
@AndrewMorton I haven't seen this before. I don't really understand it either. Also, as I said, I don't understand memory allocation and my assignment probably wouldn't be accepted if I used it. — ProGamer2711, Nov 24 '22 at 18:34
@Shawn Oh yeah! I forgot about that. So I can just check length of the different arguments in `argv` and see if they have a valid length. Am I right? — ProGamer2711, Nov 24 '22 at 18:37

Oka · Answer 1 · 2022-11-25T12:18:37.487

1

Take the addition of their strings lengths and ensure it is less than the size of your buffer.

It must be less than the size of the buffer because you must leave room for the null-terminating byte.

No dynamic memory allocation needed.

#include <stdio.h>
#include <string.h>

#define LIMIT 50

int main(int argc, char **argv)
{
    if (argc < 3) {
        fprintf(stderr, "usage: %s STRING1 STRING2\n", argv[0]);
        return 1;
    }

    if (strlen(argv[1]) + strlen(argv[2]) >= LIMIT) {
        fprintf(stderr, "Combined string length is too long.\n");
        return 1;
    }

    char result[LIMIT];
    strcpy(result, argv[1]);
    strcat(result, argv[2]);
    puts(result);
}

edited Nov 25 '22 at 12:18

answered Nov 24 '22 at 18:46

Oka

23,367
6
42
53

Why allocate extra? `size_t len = strlen(argv[1]) + strlen(argv[2]) + 1; char result[len];` – ikegami Nov 24 '22 at 19:08
@ikegami Avoiding features they did not use in their example. They said *"I don't understand memory allocation and my assignment probably wouldn't be accepted if I used it"*, so I'm not sure that VLAs would be recognized either. – Oka Nov 24 '22 at 19:10
ok, after reading the question more carefully, good point. – ikegami Nov 24 '22 at 19:10
Shouldn't the `result[LIMIT]` be `result[LIMIT+1]`? – jvx8ss Nov 25 '22 at 11:37
@jvx8ss Nope. The earlier condition ensures the combined string lengths of of the two program arguments is not greater than `LIMIT - 1`. As *separate* strings they may need up to `LIMIT + 1` total bytes, for two null-terminating bytes, but when concatenated one of the null-terminating bytes is no longer needed. (e.g., `foo\0` and `bar\0` require 8 bytes separately, their combined string lengths are 6, and their concatenated form `foobar\0` requires 7 bytes.) – Oka Nov 25 '22 at 12:15

score 0 · Accepted Answer · answered Nov 24 '22 at 19:10

As said by Shawn use the strlen() function have a look at the cs50 documentation I find it quite beginner friendly. A possible version of your code could be this:

#include <stdio.h>
#include <string.h>
int main(int argc, char *argv[]) 
{
    if (argc == 3) 
    {
        char str1[50];
        char str2[50];

        if (strlen(argv[1]) > 50 || strlen(argv[2]) > 50)
        {
           printf("Max string length is 50");
           return 1;
        }

        strcpy(str1, argv[1]);
        strcpy(str2, argv[2]);
    
        strConcat(str1, str2);

        printf("Concatenated string: %s\n", str1);
   } 

   else
   {
       printf("Invalid number of arguments passed. Format required:\n <STRING1> "
       "<STRING2>\n");
   }
}

About the concat strings. All it is doing is allocating memory with malloc and then copying the strings to the newly allocated memory while doing some checks to avoid unwanted behavior. It can be a bit confusing in the beginning, but it gets easier.

This is entirely wrong. It completely fails at the specified goal. If the string are 30 characters each, this results in undefined behaviour as 61 bytes are written to `str1`. The assignment specifically asked to catch that problem. You're checking the wrong thing entirely. — ikegami, Nov 25 '22 at 17:36
Other problems: 1) The function is called `strcat`, not `strConcat`. 2) There's no point in having `var2` and the `strcpy` into it. It's pure waste and noise. 3) You print error messages to `stdout` instead of `stderr`. 4) You return 1 on error in one place (good), but not the other (bad). 5) Over nesting / far jumps. Why is the invalid argument message so far from the check? — ikegami, Nov 25 '22 at 17:36

How do I prevent string overflow in C?

2 Answers2