Thread: Stack Smashing Detected

  1. #1
    Registered User
    Join Date
    May 2010
    Posts
    3

    Stack Smashing Detected

    My program reads IP address from files labeled 1 to 1000 ( no extensions) and then prints out the IP addresses read. Currently I have the following:


    Code:
    void clear_string(char word[50]) {
    	int x;
    	for ( x = strlen(word); x != -1; x-- ) word[x] = '\0';
    } 	
    
    int main () {
    	int i, j, k, z, t, g;
    	FILE *f;
    	char line[1000], filenumber[2], begin[15], IP[15], domain[50];
    	dlist *current, *curIP, *list, *listIP, *temp;
    
    
    	for ( i = 1; i < 1001 ;i++) {
    		sprintf(filenumber, "%d", i);
    		f = fopen(filenumber, "r"); 				// Open File
    		if ( f == NULL ) break;
    		printf("\nFile: %d\n", i);
    		listIP = NULL;
    		while ( fgets(line, 1000, f) ) { 			
    			if ( line[1] == '\n' ) break;	
    			strncpy(begin, line, 14);
    			if ( begin[sizeof(begin)-1] != '\0' ) begin[sizeof(begin)-1] = '\0';		
    			if ( strcmp( "Received: from", begin ) == 0 ) { // See if IP Adress is in this line
    				for ( j = 0, k = 0; j < strlen(line) ; j++ ) {
    					if ( line[j] == '.' || isdigit(line[j]) != 0 )  {
    						IP[k] = line[j];
    						k++;
    					} else if ( k < 7 ) {
    						k = 0;
    						clear_string(IP);
    					} else if ( k >= 7 && k <= 15 && correct_IP_format(IP) == 1 ) {
    						for ( t = 0, temp = listIP; temp != NULL; temp = temp->next) {
    							if ( strcmp(temp->val, IP) == 0 ) t = 1;
    						}
    						if ( t == 0 ) {
    							curIP = (dlist *)malloc(sizeof(dlist));
    							strcpy(curIP->val, IP);
    							curIP->next = listIP;
    							listIP = curIP;
    						}
    						break;
    					}
    				}
    		//	}
    
    		}
    		for ( curIP = listIP; curIP != NULL; curIP = curIP->next ) {
    			printf("IP: %s\n", curIP->val);
    		}
    		fclose(f);
    	}
    }
    After I enter the while loop that extracts each line from the file, I extract the first 14 characters from that line to see if it begins with "Recieved: from". This is because I noticed a pattern that the IP addresses are located within the same line. That was a while ago and I have since noticed this is not the case and that I need to check every line for an IP address regardless of what it starts with.

    I should note the above code works and there are no issues with it. Once I remove the lines of code that check to see if the current line of the file begins with "Recieved: from" is when problems begin. Those lines being:
    Code:
    strncpy(begin, line, 14);
    if ( begin[sizeof(begin)-1] != '\0' ) begin[sizeof(begin)-1] = '\0';		
    if ( strcmp( "Received: from", begin ) == 0 ) { // See if IP Adress is in this line
    and their respective closing brackets.

    So the final piece of code that is giving me stack smashing issues is the following:


    Code:
    void clear_string(char word[50]) {
    	int x;
    	for ( x = strlen(word); x != -1; x-- ) word[x] = '\0';
    } 	
    
    int main () {
    	int i, j, k, z, t, g;
    	FILE *f;
    	char line[1000], filenumber[2], begin[15], IP[15], domain[50];
    	dlist *current, *curIP, *list, *listIP, *temp;
    
    
    	for ( i = 1; i < 1001 ;i++) {
    		sprintf(filenumber, "%d", i);
    		f = fopen(filenumber, "r"); 				// Open File
    		if ( f == NULL ) break;
    		printf("\nFile: %d\n", i);
    		listIP = NULL;
    		while ( fgets(line, 1000, f) ) { 			
    			if ( line[1] == '\n' ) break;	
    			for ( j = 0, k = 0; j < strlen(line) ; j++ ) {
    				if ( line[j] == '.' || isdigit(line[j]) != 0 )  {
    					IP[k] = line[j];
    					k++;
    				} else if ( k < 7 ) {
    					k = 0;
    					clear_string(IP);
    				} else if ( k >= 7 && k <= 15 && correct_IP_format(IP) == 1 ) {
    					for ( t = 0, temp = listIP; temp != NULL; temp = temp->next) {
    						if ( strcmp(temp->val, IP) == 0 ) t = 1;
    					}
    					if ( t == 0 ) {
    						curIP = (dlist *)malloc(sizeof(dlist));
    						strcpy(curIP->val, IP);
    						curIP->next = listIP;
    						listIP = curIP;
    					}
    					break;
    				}
    			}
    		}
    		for ( curIP = listIP; curIP != NULL; curIP = curIP->next ) {
    			printf("IP: %s\n", curIP->val);
    		}
    		fclose(f);
    	}
    	
    }
    I should also note that my program executes perfectly fine and everything works all the way to the end at which point it gives me the stack smashing error. It doesnt make much sense to me because those 3 lines are just extracting the first 14 characters of the line variable to the begin variable and then checking if the begin variable equals "Recieved: from". I do not use the begin variable anywhere else so I dont see what the issue is.

    Any ideas or tips on how to debug it?

    Thank you
    Last edited by halexh; 05-18-2010 at 10:12 AM.

  2. #2
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    > sprintf(filenumber, "%d", i);
    Your buffer is only good for 0 to 9
    But you need at least 5 chars to store "1000\0"

    > if ( line[j] == '.' || isdigit(line[j]) != 0 )
    This does not check that there is still room in your IP variable.

    > clear_string(IP);
    If you made sure your IP string was always a proper string when you construct it, there would be no need to have this.
    Further, you're relying on it to BE a proper string to begin with - you call strlen().
    If this isn't the case, you really do run the risk of trashing a lot of memory.
    It would be a lot better to simply have
    memset( IP, 0, sizeof(IP) );

    > I should note the above code works and there are no issues with it.
    This is almost never true when people make such bold statements.
    Apparently correct output + no crashes should NEVER be taken as meaning bug-free.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  3. #3
    Registered User
    Join Date
    May 2010
    Posts
    3
    Thank you Salem. I have it working now and it looks cleaner because of that memset suggestion.

  4. #4
    Registered User
    Join Date
    May 2010
    Posts
    3
    I have another question related to the above program. As you can see I am assuming that the files I am reading from are in the same directory as the program. Is it possible to redirect a directory that contains the files and open them that way?

    For example

    ./progname < directoryoffiles

  5. #5
    Registered User
    Join Date
    Dec 2007
    Posts
    2,675
    No...you can pass the directory name as an argument to the program and use that along with the dir functions (opendir, readdir, etc.), but you can't redirect as you show. AFAIK.

  6. #6
    spurious conceit MK27's Avatar
    Join Date
    Jul 2008
    Location
    segmentation fault
    Posts
    8,300
    Quote Originally Posted by halexh View Post
    I have another question related to the above program. As you can see I am assuming that the files I am reading from are in the same directory as the program. Is it possible to redirect a directory that contains the files and open them that way?

    For example

    ./progname < directoryoffiles
    With the bash (*nix) shell command line:

    Code:
    for x in directoryoffiles/*; do ./progname < $x; done
    C programming resources:
    GNU C Function and Macro Index -- glibc reference manual
    The C Book -- nice online learner guide
    Current ISO draft standard
    CCAN -- new CPAN like open source library repository
    3 (different) GNU debugger tutorials: #1 -- #2 -- #3
    cpwiki -- our wiki on sourceforge

  7. #7
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    If directoryoffiles is just a text file listing of file paths you want to read from, then that's perfectly do-able.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  8. #8
    Registered User jeffcobb's Avatar
    Join Date
    Dec 2009
    Location
    Henderson, NV
    Posts
    875
    Quote Originally Posted by Salem View Post
    If directoryoffiles is just a text file listing of file paths you want to read from, then that's perfectly do-able.
    No at least on UNIX he can read from stdin and still use
    Code:
    myprog < /path/to/files/
    which would feed him the directory name so he could use that as a prefix to his filenames. What I think he wants is to read the filenames that way and
    Code:
    myprog < ls /path/to/files/
    when combined with stdin will work just fine. This is a very common UNIX method of chaining the output of app1 to app2. Also he could use command line switches to get the directory of files w/o readying stdin directly, which is a facility that is lost by doing the int main(void) nonsense...
    C/C++ Environment: GNU CC/Emacs
    Make system: CMake
    Debuggers: Valgrind/GDB

  9. #9
    Registered User
    Join Date
    Sep 2006
    Posts
    8,868
    Jeff, could you expand on what you mean, by this?

    ... he could use command line switches to get the directory of files w/o readying stdin directly, which is a facility that is lost by doing the int main(void) nonsense...
    I was with 'ya, right up to this sentence. Thanks Jeff!

  10. #10
    Registered User jeffcobb's Avatar
    Join Date
    Dec 2009
    Location
    Henderson, NV
    Posts
    875
    Quote Originally Posted by Adak View Post
    Jeff, could you expand on what you mean, by this?



    I was with 'ya, right up to this sentence. Thanks Jeff!
    Well I am not sure which part you are responding to but

    Re getting the directory from the command line switched: If int main(void) was changed to int main(int argc, char *argv[]) the programmer could then (at a minimum) add the following the the first few lines of the file:
    Code:
    int main(int argc, char *argc[])
    {
         string strDirectoryToSearch = "./";
         if(argc >= 2)
         {
              strDirectoryToSearch = argv[1]
    
         }
    
    }
    so when the program is called like:
    myprog /path/to/search
    Then strDirectoryToSearch will contain his path and from this he could use standard directory walking code (which I can supply if need-be) to make full paths and then he could iterate though them to process each file that fits a pattern.

    If you are talking about my state about Windows coders dismissing the int argc, char *argv, most times on here you can get the moderators saying int main(void) is OK, effectively discarding and chance of what the OP wanted. It is a UNIX mindset (see the Art of Computer Programming by ERS) the best programs have two key features:
    1. Every program should be small and accomplish one goal and do it well.
    2. Every program should accept stdin input and output stuff in a format that another program can read.

    This combination of rules is what lets one do this:
    Code:
    find /usr/include -name \*.h | xargs grep INT | grep MAX
    Which calls find, looking in /usr/incude for files that match the pattern *.h.
    This then is passed to xargs which called the app grep after that. This results in every file in /usr/include/*.h being searched for any lines that contain the string "INT", the output of this is sent to another instance of grep which searches for MAX.

    int main(void) defeats much of this.

    If you are wondering about reading stdout I can provide source but I will only do it if someone asks. I just did it same thing last Friday at work, albeit with Python..
    C/C++ Environment: GNU CC/Emacs
    Make system: CMake
    Debuggers: Valgrind/GDB

  11. #11
    Registered User
    Join Date
    Sep 2006
    Posts
    8,868
    Thanks for the explanation, Jeff. I don't have any problem dealing with searching through files or their contents. Your comment just left me wondering.

    I understand (and I hope it's correct), that int main(void) is just a shortcut for correct programs that will not be using the argc, *argv[] features, and doesn't mean to imply that these parameters don't exist, or should be ignored in other programs.

    But THIS, ** THIS ** is unforgivable:

    I just did it same thing last Friday at work, albeit with Python..
    All I can suggest is pleading temporary insanity and throwing yourself on the mercy of the forum!

  12. #12
    Registered User jeffcobb's Avatar
    Join Date
    Dec 2009
    Location
    Henderson, NV
    Posts
    875
    Quote Originally Posted by Adak View Post
    Thanks for the explanation, Jeff. I don't have any problem dealing with searching through files or their contents. Your comment just left me wondering.

    I understand (and I hope it's correct), that int main(void) is just a shortcut for correct programs that will not be using the argc, *argv[] features, and doesn't mean to imply that these parameters don't exist, or should be ignored in other programs.

    But THIS, ** THIS ** is unforgivable:



    All I can suggest is pleading temporary insanity and throwing yourself on the mercy of the forum!
    Well this was the requirement of the job (scripting language). The only reason I brought it up is to show that the same basic tactic of reading stdin is a common one. When it comes to Python, well, I would rather code in that rather than Perl and I don't know Ruby well enough to code it "on the clock".
    C/C++ Environment: GNU CC/Emacs
    Make system: CMake
    Debuggers: Valgrind/GDB

  13. #13
    Registered User
    Join Date
    Sep 2006
    Posts
    8,868
    Your lawyer can use that -- obvious extenuating circumstances!!

    Now, was there any liquor involved? He may still need further signs of temporary incapacity! << ROFL>>

  14. #14
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,318
    Quote Originally Posted by halexh View Post
    My program reads IP address from files labeled 1 to 1000 ( no extensions) and then prints out the IP addresses read.
    Should we be concerned that you have lists of thousands of IP addresses?
    Is there a logical explanation behind that?
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. *** stack smashing detected ***
    By chakra in forum C Programming
    Replies: 2
    Last Post: 06-09-2009, 09:12 PM
  2. *** stack smashing detected ***
    By Martin_HS in forum C Programming
    Replies: 9
    Last Post: 05-29-2009, 04:01 AM
  3. stack and pointer problem
    By ramaadhitia in forum C Programming
    Replies: 2
    Last Post: 09-11-2006, 11:41 PM
  4. Question about a stack using array of pointers
    By Ricochet in forum C++ Programming
    Replies: 6
    Last Post: 11-17-2003, 10:12 PM
  5. error trying to compile stack program
    By KristTlove in forum C++ Programming
    Replies: 2
    Last Post: 11-03-2003, 06:27 PM