Remove security flaws in my code with help of splint

168 views Asked by At

Can anybody help me to fix this code? I don't understand much as I'm new to C. I'm using Splint to find security flaws in the code.

char *stringcopy(char *str1, char *str2)
{
    while (*str2)
        *str1++ = *str2++;

    return str2;
}

main(int argc, char **argv)
{

    char *buffer = (char *)malloc(16 * sizeof(char));

    stringcopy(buffer, argv[1]);

    printf("%s\n", buffer);
}

Splint output

2

There are 2 answers

0
Paul Ogilvie On

Your stringcopy does not terminate the copied string. Besides, there is not much use in returning the end of the source string. The following is a suggestion:

char *stringcopy(char *str1, char *str2)
{
    char *s2= str2;
    while (*s2)
        *str1++ = *s2++;

    *s2= '\0';
    return str2;
}
0
Deduplicator On
  1. You are missing the includes.
  2. Your stringcopy() does not terminate the destination.
  3. The source should really be a const char* to allow for const-correctness and let the compiler help catch bugs.
  4. stringcopy() expects the destination to be big enough. Does 16 Bytes qualify for that in main()?
    Consider packaging allocating and copying a string into one function, well-known as strdup().
  5. For some reason, standard strcpy() returns a pointer to the destination. Yes, it's a good idea to return a pointer to the trminator instead, but consider following existing practice when naming the function to avoid unpleasant surprises.
  6. Do not cast the result of malloc().
  7. Additionally, use sizeof *pointer instead of sizeof(TYPE), it reduces unchecked repetition and avoids bugs.
  8. Do not assume success. malloc() can always fail.
  9. Generally, you should free() what you malloc(). As the program terminates immediately though, it would be useless make-work.
  10. Implicit int is deprecated in C90, and removed in C99.
  11. Implicit return 0; for main() appeared in C99, earlier it was Undefined Behavior (UB). But you already used implicit int, which was removed at that time. What shall it be?
  12. Even though this isn't code-review, I earnestly recommend you put a bit more work into properly naming your parameters. Anyone reading your code for any reason (which is mostly you yourself at the moment) will be thankful. That does not mean the names should be longer though.