Check if input string exceeds buffer limit (crash)

Question

I'm creating a function that will pass a directory path as an argument or if it is left blank, prompt the user for an input.

I have set my PATH_MAX=100 and if statements to check if ((strlen(folder path) + strlen(file path)) > PATH_MAX) will ask the user to input again.

However when I was checking that all conditions work (set PATH_MAX=20), if folder path exceeds by itself the PATH_MAX, buffer crashes due to insufficient size (L'Buffer is too small' &&0).

Is there a way to check whether the user exceeds the PATH_MAX beforehand and inform that path is too long, in order to avoid crashing the buffer? Or should i just increase the size of PATH_MAX?

Code:

#define PATH_MAX 100
void CreateFiles(char folder[PATH_MAX])
{
    char addrbook[PATH_MAX] = "caf-sorted.txt";
    char path[PATH_MAX]="";

    if ((strlen(folder)<4)) 
    {
        //User inputs directory
        printf("Enter New Directory\n(!Do not enter filename!)\n");

        if (NULL == fgets(path, sizeof path, stdin))
        {//check if fgets fails
            if (ferror(stdin))
            {
                folder="";
                perror("fgets() failed");
                CreateFiles(folder);
                return;
            }
        }
    }
    else
        memcpy(path, folder, strlen(folder));

    path[strcspn(path, "\n\r")] = 0;

    if (strlen(addrbook) > 0 && '\\' != path[strlen(path) - 1])
    {
        if (PATH_MAX < strlen(path))
        {
            errno = EINVAL;
            perror("'path' too long");
            folder="";
            CreateFiles(folder);
            return;
        }

        strcat(path, "\\");
    }

    if (PATH_MAX < (strlen(path) + strlen(addrbook)))
    {
        errno = EINVAL;
        perror("'path\\filename' too long");
        folder="";
        CreateFiles(folder);
        return;
    }
}

You should check it in the place where you are getting it from. — Eugene Sh.
– Eugene Sh., Commented Apr 28, 2017 at 15:25
Even with CreateFiles(char folder[PATH_MAX]), folder could pointer to a string longer than MAX_PATH - this can fail memcpy(path, folder, strlen(folder));. — chux
– chux, Commented Apr 28, 2017 at 15:29
folder is being checked in main before passing in CreateFiles, if (folder>PATH_MAX){folder="";} — Jimmy_A
– Jimmy_A, Commented Apr 28, 2017 at 15:33
path[strlen(path) - 1] is an exploitable hack as strlen(path) is not guaranteed to be > 0. — chux
– chux, Commented Apr 28, 2017 at 15:34
if (folder>PATH_MAX) compares a pointer to an integer - perhaps you meant some other test? IAC, better that CreateFiles() stands on its own and does not fail should a pointer to a long string get passed in. — chux
– chux, Commented Apr 28, 2017 at 15:37

afdggfh · Accepted Answer · 2017-04-28 16:52:05Z

2

You have to take the terminating null character into account

if (!(strlen(path) < PATH_MAX))

makes sure that the count of characters in path (without the null character) is always at least one less than PATH_MAX, which leaves space for the terminating null character.

You have to take that into account with every C string you use, since strlen(char *string) is always one LESS than the space you need to store the string if you want to be able to null-terminate it.

Edit: So I've looked into at least the first few lines of your function and tried to re-implement them quickly. It's not beautiful, but it works:

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

#define PATH_MAX 100

void create_files (char *folder)
{
    char addr_book[] = "caf-sorted.txt";
    char path[PATH_MAX];


    // Setting all bytes in *path to zero
    bzero(path, PATH_MAX);



    // If the folder name is too short, we ask for a new one
    if (strlen(folder) < 4) {

        char c; // This will store our input from stdin, one char at a time



        // As long as the supplied path name is too short, we'll keep asking:

        while (strlen(path) < 4) {
            printf("Please enter a path (no filename!): ");


            // We get one character at a time from stdin using getc(...):
            // until we encounter a newline

            for (int i = 0; (c = getc(stdin)) != '\n'; i++) {

                if (i < PATH_MAX - 1) {

                    // As long as we have space for two more characters
                    // (the value of c plus a null character after it)
                    // We'll keep appending c:

                    path[i] = c;

                } else if (i == PATH_MAX - 1) {

                    // If we get too many characters from stdin, we
                    // display an error message and reset our path to
                    // all null characters again, so the outermost loop
                    // will run again

                    fprintf(stderr, "Path is too long!\n");
                    bzero(path, PATH_MAX);

                    // Notice that we do not have a break statement
                    // here, we iterate through the input string from
                    // stdin until we encounter a newline character,
                    // so we don't have any superfluous characters
                    // that spill into the beginning ouf our freshly
                    // reset path string
                }
            }
        }
    } else {
        // Or, you know, if the programmer specifies a proper value,
        // Just do it the easy way and copy that into our path string
        // (although this will truncate a folder name that is too long):

        strncpy(path, folder, PATH_MAX - 1);
    }
}

int main ()
{
    create_files("");

    return 0;
}

edited Apr 28, 2017 at 16:52

answered Apr 28, 2017 at 15:25

afdggfh

1359 bronze badges

Sign up to request clarification or add additional context in comments.

16 Comments

Jimmy_A Over a year ago

The thing is that the buffer crashes in the fgets command. If it was passed that stage my if statements would have captured it. So basically I have to find a way so that fgets doesn't exceed the limit

afdggfh Over a year ago

fgets(path, PATH_MAX, stdin); Same principle. You might want to set all the bytes in path to zero first, like this: bzero(path, PATH_MAX) for this you'll need to #include <strings.h>

Jimmy_A Over a year ago

So the best approach would be limiting fgets to accept 99 characters? I new that, but imagined there would be a better approach. string.h is already included. Couldn't write down every line, there are too many:)

afdggfh Over a year ago

This might be confusing, because there is <string.h> (singular) and <strings.h> (plural). This is how it is on my system, anyways. And I just looked at the fgets(...) man page, it already seems to read size - 1 bytes. I'll dig into the code and see what I can find.

chux Over a year ago

@afdggfh "You might want to set all the bytes in path to zero first" --> OP's code all ready does that with char path[PATH_MAX]="";

|

Collectives™ on Stack Overflow

Check if input string exceeds buffer limit (crash)

1 Answer 1

16 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

16 Comments

Your Answer

Sign up or log in

Post as a guest

Related