Thread: Coding a check for array overflow? or how to have no input limit.

  1. #1
    Registered User
    Join Date
    Aug 2015
    Posts
    11

    Coding a check for array overflow? or how to have no input limit.

    Hi guys,

    I'm about 4 weeks in to a beginners C course I'm taking online, the information is being given but it seems they never really touch more than the surface.

    currently they are saying getting a string is an issue because of array size, and overflowing this causes segmentation faults.

    they havent said how to get around this so I've been trying to figure it out as per my code below :

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    
    int main(void)
    {
    //array with 20 char spaces
    char s[20];
    
    do
        // take user input including white space
        {
        scanf("%[^\n]",s);
        }
    // while s is empty
    while ( s == NULL);
    
    // convert s to an int so I can check its not > array size
    int count = atoi(s);
    
    if (count > 20)
        {
        // return erro code
        return 1;
        }
    
    else
        {
        // print input to screen
        printf("%s\n",s);
        }
    
    return 0;
    }
    unsure as to why this isnt working, should I be using a linked list for an input string? we have only just started to look at linked lists.

    please help, thanks.

  2. #2
    Registered User dariyoosh's Avatar
    Join Date
    Nov 2012
    Location
    Iran / France
    Posts
    38
    Hi,

    Based on the line 7, of your example
    Code:
    char s[20];
    The max length is known.

    Also I don't understand the use of atoi in your code. You find the length of the string by using strlen (see below)

    When you know how many characters you can read (at most), you can pass this information to scanf. Here is an example that reads at most 5 characters:
    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    
    int main(int argc, char *argv[])
    {
        char text[5];
        
        
        scanf("%5s",text);
        
        printf("The text you entered is: %s\n", text);
        printf("The length of your text = %zu\n", strlen(text));
        
        return EXIT_SUCCESS;
    }

  3. #3
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    Quote Originally Posted by MonkeyTennis
    they havent said how to get around this so I've been trying to figure it out as per my code below
    Have you learnt about pointers and how to use malloc and free?

    dariyoosh: since you declared text to be an array of 5 char, the format for scanf should be "%4s".
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  4. #4
    Registered User rstanley's Avatar
    Join Date
    Jun 2014
    Location
    New York, NY
    Posts
    1,106
    You have many problems here.

    You don't initialize s. It will pick up garbage in memory.
    You are not capturing the return value from scanf(), to check if scanf() actually returned a valid string.
    Code:
    while ( s == NULL);
    s will NEVER be NULL! s is always the address of the first char in the array!
    Code:
    int count = atoi(s);
    atoi retuns the string in s converted to an integer, NOT the length of the string entered. If I typed in "123" then count would be the integer 123, not 3!
    You need to use strlen(), NOT atoi().
    Code:
    if (count > 20)
    If I had typed in "123" then the if statement would be true. You are trying to test if the user typed in more than 20 characters, but by then, if I had, you would have overwritten beyond the end of the array!
    There is a way to do it correctly with scanf() but better to use fgets() to input a string of maximum 19 characters, allowing room for a Nul byte, '\0'.

  5. #5
    Registered User dariyoosh's Avatar
    Join Date
    Nov 2012
    Location
    Iran / France
    Posts
    38
    Quote Originally Posted by laserlight View Post
    dariyoosh: since you declared text to be an array of 5 char, the format for scanf should be "%4s".
    According to man 3 scanf, I don't think that 4 is the right value but rather 5
    ...
    An optional decimal integer which specifies the maximum field width. Reading of characters stops either when this maximum is reached or when a nonmatching character is found, whichever happens first. Most
    conversions discard initial white space characters (the exceptions are noted below), and these discarded characters don't count toward the maximum field width. String input conversions store a terminating
    null byte ('\0') to mark the end of the input; the maximum field width does not include this terminator.
    ...
    And here is the output of the example I provided:

    Code:
    $ gcc -Wall testprog.c -o testprog
    $ ./testprog
    hellooooooooooooooooo
    The text you entered is: hello
    The length of your text = 5
    $

  6. #6
    Registered User rstanley's Avatar
    Join Date
    Jun 2014
    Location
    New York, NY
    Posts
    1,106
    The text you entered is: hello
    The length of your text = 5
    The null byte was stored OUTSIDE the end of the array, NOT inside at text[4], the last byte in the array.

  7. #7
    Registered User
    Join Date
    May 2010
    Posts
    4,633
    According to man 3 scanf, I don't think that 4 is the right value but rather 5
    No, look closely at that quote you provided, the part you highlighted is the key, along with this:
    Reading of characters stops either when this maximum is reached
    You must reserve room for the end of string character, which scanf() always adds to a string.

    Jim

  8. #8
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    Quote Originally Posted by dariyoosh
    According to man 3 scanf, I don't think that 4 is the right value but rather 5
    No, 4 is the right value. Read carefully the part that you bolded: if the field width specified does not include the null terminator, then we have to account for the null terminator. So, if you specify the field width as 5, then a maximum of 5 characters could be read into the array. So, with the null terminator to make it a null terminated string, this means that the array must have a minimum of 6 characters. But the array was declared as having 5 characters, hence a buffer overflow becomes possible.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  9. #9
    Registered User dariyoosh's Avatar
    Join Date
    Nov 2012
    Location
    Iran / France
    Posts
    38
    Quote Originally Posted by jimblumberg View Post
    No, look closely at that quote you provided, the part you highlighted is the key, along with this:

    You must reserve room for the end of string character, which scanf() always adds to a string.

    Jim
    I see now my mistake, thanks for the explanation.

  10. #10
    Registered User dariyoosh's Avatar
    Join Date
    Nov 2012
    Location
    Iran / France
    Posts
    38
    Quote Originally Posted by laserlight View Post
    No, 4 is the right value. Read carefully the part that you bolded: if the field width specified does not include the null terminator, then we have to account for the null terminator. So, if you specify the field width as 5, then a maximum of 5 characters could be read into the array. So, with the null terminator to make it a null terminated string, this means that the array must have a minimum of 6 characters. But the array was declared as having 5 characters, hence a buffer overflow becomes possible.
    Thanks for pointing this out.

  11. #11
    Registered User
    Join Date
    Aug 2015
    Posts
    11
    Hi guys, thanks for pointing these things out to me, cant believe I tried to use atoi and not bloody strlen!

    Yes we have been fought pointers, however I understand how they work but not how to use them properly, that's the problem with cs50 through edx, its very disjointed. Should I be allocating s with a pointer and malloc before setting input to s?

    We had "training wheels" on up untill this week which handled scanf and everything else, all we had to put was getstring() and it would handle all the allocation etc, I'm struggling to implement simple things now.

  12. #12
    Registered User rstanley's Avatar
    Join Date
    Jun 2014
    Location
    New York, NY
    Posts
    1,106
    For a program this simple, I would not bother with using malloc() and free().

    Unfortunately, a course that provides "A broad and robust understanding of computer science and programming" cannot teach you a thorough understanding of the C Language, that you would need to properly use the language. This also goes for "PHP, and JavaScript plus SQL, CSS, and HTML" that the course introduces to the student. You would need to follow up with a full course in C and/or any other language you wish to learn.

  13. #13
    Registered User
    Join Date
    Aug 2015
    Posts
    11
    is this better? is this all I have to do every time I need to store user input?

    Code:
    #include <stdio.h>
    #include <string.h>
    #include <stdlib.h>
    
    int main(void)
    {
    //array with 50 char spaces
    char* s = malloc(50 * sizeof(char));
    
    do
        // take user input including white space
        {
        printf("please enter some text no longer than 50 chars:\n");
        scanf("%[^\n]",s);
        }
        
    // while s is empty or over 50
    while ( strlen(s)+1 == 0 || strlen(s)+1 > 50 );
    
    if (s != NULL)
        {
        // print input to screen
        printf("\nYou typed:\n");
        printf("%s\n",s);
        }
        
    else
        {
        free(s);
        return 1;
        }   
         
    free(s);
    return 0;
    }
    Last edited by MonkeyTennis; 09-11-2015 at 02:57 PM.

  14. #14
    Registered User
    Join Date
    May 2010
    Posts
    4,633
    Why the malloc? Why not just use a statically allocated array?

    Never use a function that doesn't limit the number of characters it will try to retrieve into a string. When using scanf() always use the "optional" width specifier to limit the number of characters.

    Look at the following:

    Code:
     
    //array with 50 char spaces
    char* s = malloc(50 * sizeof(char));
    ...
       {
        printf("please enter some text no longer than 50 chars:\n");
        scanf("%[^\n]",s);
    Your comment and the printf is incorrect. Since the size of the array is 50 you can enter at most 49 characters into your string. You must always reserve room for the end of string character ('\0'). Your scanf() should look like:

    Code:
       scanf("%49[^\n", s);
    Although in this case I would probably suggest using fgets() instead.
    Code:
     fgets(s, 50, stdin);
    Jim

  15. #15
    Registered User rstanley's Avatar
    Join Date
    Jun 2014
    Location
    New York, NY
    Posts
    1,106
    The use of malloc() really does not help you here. Many of the problems still exist in the code.
    Code:
    char* s = malloc(50 * sizeof(char));
    "sizeof(char)" is ALWAYS 1, and only 1.
    Code:
    char* s = malloc(50);
    You should also check if the return from malloc() was successful.
    Code:
    do
        // take user input including white space
        {
        printf("please enter some text no longer than 50 chars:\n");
        scanf("%[^\n]",s);
        }
         
    // while s is empty or over 50
    while ( strlen(s)+1 == 0 || strlen(s)+1 > 50 );
    If the user has typed in more than 49 characters, you're too late! You have overwritten memory outside of the allocated memory!
    A better solution:
    Code:
    scanf("%49[^\n]", s);
    Or better use fgets().
    "strlen(s)+1 == 0" strlen() returns 0 or greater than 0, "strlen(s)+1" will NEVER equal zero!
    Code:
    if (s != NULL)
        {
        // print input to screen
        printf("\nYou typed:\n");
        printf("%s\n",s);
        }
    You are not checking if someone typed in a string successfully, you are only checking that malloc() returned a valid address of the allocated memory!

    I think you need to pick up a good book on C that can fill in the gaps that the course left WIDE open.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. How do I check what causes a stack overflow?
    By MutantJohn in forum C++ Programming
    Replies: 11
    Last Post: 09-24-2013, 05:00 PM
  2. Limit input to 10 numbers
    By Qshine in forum C Programming
    Replies: 5
    Last Post: 02-11-2013, 09:19 PM
  3. time limit for user input
    By dudeomanodude in forum C++ Programming
    Replies: 4
    Last Post: 07-18-2008, 03:01 PM
  4. Overflow check
    By tezcatlipooca in forum C Programming
    Replies: 5
    Last Post: 12-23-2006, 12:04 PM
  5. How to check 32 bit limit
    By maven in forum C Programming
    Replies: 3
    Last Post: 09-28-2006, 02:47 AM