failing with certain string lengths

This is a discussion on failing with certain string lengths within the C Programming forums, part of the General Programming Boards category; well, I've just now written the below little program. it is a Win32 app, however my question doesn't regard anything ...

  1. #1
    Madly in anger with you
    Join Date
    Nov 2005
    Posts
    211

    failing with certain string lengths

    well, I've just now written the below little program. it is a Win32 app, however my question doesn't regard anything Win32 specific.

    this program will produce some funky results if the string passed is too small to hold the entire IP address. it works fine if the string passed is large enough, and most of the time even if it is too small, however not all the time. my goal with the string length here is to prevent a buffer overflow, however it's not exactly doing that for every case.

    when I declare the string szStr of different sizes, specifically 1, 4, 8, 11, and 12 bytes it crashes. I should also note that I get weird results with values less than 12, I seem to somehow get more characters outputted than what are supposed to be.

    Code:
    #include <string.h>
    #include <ctype.h>
    #include <windows.h>
    #include <winsock.h>
    
    char *GetIP(char *pszDest, char *pszDesc, char *pszBuf, int iLen, unsigned short usPort)
    {
    	SOCKET sock;
    	WSADATA wsaD;
    	SOCKADDR_IN sin;
    	PHOSTENT pHostEnt;
    	char szGET[40], szBuf[1024], *p, *ip;
    	int i, j;
    
    	WSAStartup(0x101, &wsaD);
    
    	sock = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
    
    	if(!(pHostEnt = gethostbyname(pszDest)))
    		goto Failure;
    
    	sin.sin_family = AF_INET;
    	sin.sin_addr = *((PIN_ADDR)pHostEnt->h_addr);
    	sin.sin_port = htons(usPort);
    
    	if(connect(sock, (PSOCKADDR)&sin, sizeof(SOCKADDR_IN)) == SOCKET_ERROR)
    		goto Failure;
    
    	wsprintf(szGET, "GET / HTTP/1.1\r\nHost: &#37;s\r\nConnection: close\r\n\r\n", pszDest);
    
    	if(send(sock, szGET, strlen(szGET) + 1, 0) == SOCKET_ERROR)
    		goto Failure;
    
    	do {
    		i = recv(sock, szBuf, sizeof szBuf - 1, 0);
    		
    		if(p = strstr(szBuf, pszDesc)) {
    			ip = p + strlen(pszDesc);
    
    			for(j = 0; isdigit(*ip) || *ip == '.' && j < iLen; ip++, j++)
    				*(pszBuf + j) = *ip;
    
    			*(pszBuf + j) = '\0';
    		}
    	} while(i > 0 && i != SOCKET_ERROR);
    
    	closesocket(sock);
    	WSACleanup();
    
    	return pszBuf;
    Failure:
    	closesocket(sock);
    	WSACleanup();
    
    	return NULL;
    }
    
    int WinMainCRTStartup()
    {
    	char szStr[4];
    
    	MessageBox(NULL, GetIP("whatismyip.com", "Your IP Is ", szStr, sizeof szStr - 1, 80), "hi", MB_OK);
    
    	return 0;
    }
    could anyone here please tell me what is happening here? thank you in advance.
    Last edited by Bleech; 06-01-2007 at 01:55 PM.

    Intel Core 2 Quad Q6600 @ 2.40 GHz
    3072 MB PC2-5300 DDR2
    2 x 320 GB SATA (640 GB)
    NVIDIA GeForce 8400GS 256 MB PCI-E

  2. #2
    Gawking at stupidity
    Join Date
    Jul 2004
    Location
    Oregon, USA
    Posts
    3,163
    Code:
    sizeof szBuf
    Since szBuf is a pointer, using sizeof on it will give you the size of the pointer (typically 4 bytes). If you want to find out how many bytes are allocated for szBuf, you'll need to pass that size as another argument to your function.
    If you understand what you're doing, you're not learning anything.

  3. #3
    Madly in anger with you
    Join Date
    Nov 2005
    Posts
    211
    hmm. I don't think that's the problem though, at least, I can't see how it would relate to the results that I'm recieving. I'm not having trouble sending and recieving data, I am having trouble parsing the data once it has been recieved.

    for example, the above code will print:

    11.222

    (fake IP for privacy, 7 bytes (with NULL?), don't ask how that is being contained in szStr)

    I should also note that if I try compiling this in Debug build config and run it, it will trigger an error message asking to start it in the debugger. when I step into the debugger, here are some messages I recieve:

    test.exe has triggered a breakpoint
    After I hit "Continue":

    Run-Time Check Failure #2 - Stack around the variable 'szGET' was corrupted.
    A string looking like the above (11.222) is outputted in a message box, and then:

    Run-Time Check Failure #2 - Stack around the variable 'szStr' was corrupted.
    I think it's probably an error in my string manipulation, specifically this block of code:

    Code:
    if(p = strstr(szBuf, pszDesc)) {
    			ip = p + strlen(pszDesc);
    
    			for(j = 0; isdigit(*ip) || *ip == '.' && j < iLen; ip++, j++)
    				*(pszBuf + j) = *ip;
    
    			*(pszBuf + j) = '\0';
    		}
    although I just can't find the problem. any help is appreciated. thanks again in advance.

    Intel Core 2 Quad Q6600 @ 2.40 GHz
    3072 MB PC2-5300 DDR2
    2 x 320 GB SATA (640 GB)
    NVIDIA GeForce 8400GS 256 MB PCI-E

  4. #4
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,304

    Arrow

    Hmm, spot the VB programmer!

    (when else does one use gotos like 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"

  5. #5
    Madly in anger with you
    Join Date
    Nov 2005
    Posts
    211
    actually, I don't code VB, and I never have. I normally don't use goto statements, but here they saved me alot of repetition. although the goto statements are also repetitive, they're smaller/shorter than repeating the code in the Failure label.

    back on topic, this problem is driving me crazy.

    Intel Core 2 Quad Q6600 @ 2.40 GHz
    3072 MB PC2-5300 DDR2
    2 x 320 GB SATA (640 GB)
    NVIDIA GeForce 8400GS 256 MB PCI-E

  6. #6
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,541
    > wsprintf(szGET, "GET / HTTP/1.1\r\nHost: &#37;s\r\nConnection: close\r\n\r\n", pszDest);
    This is already 45 characters long, before you add anything with the %s substitution.
    Instant buffer overflow.

    > i = recv(sock, szBuf, sizeof szBuf - 1, 0);
    1. Check that i is > 0 before doing anything with the buffer.
    2. recv() doesn't append a \0 (unless you send one).
    3. Since both send() and recv() can fragment messages, you can't rely on point 2 working for you.

    Something like
    Code:
    i = recv(sock, szBuf, sizeof szBuf - 1, 0);
    if ( i > 0 ) {
        szBuf[i] = '\0';
        // Now you can use str... functions.
    }
    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.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  7. #7
    Sweet
    Join Date
    Aug 2002
    Location
    Tucson, Arizona
    Posts
    1,807
    Are you sure your szGET buffer is enough for the string you are trying to put into it?
    Code:
    #include <string.h>
    #include <ctype.h>
    #include <windows.h>
    #include <winsock.h>
    
    char *GetIP(char *pszDest, char *pszDesc, char *pszBuf, int iLen, unsigned short usPort)
    {
    	SOCKET sock;
    	WSADATA wsaD;
    	SOCKADDR_IN sin;
    	PHOSTENT pHostEnt;
    	char *szGET, szBuf[1024], *p, *ip;
    	int i, j;
    	size_t getBufSize = 0;
    
    	WSAStartup(0x101, &wsaD);
    
    	sock = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
    
    	if(!(pHostEnt = gethostbyname(pszDest)))
    		goto Failure;
    
    	sin.sin_family = AF_INET;
    	sin.sin_addr = *((PIN_ADDR)pHostEnt->h_addr);
    	sin.sin_port = htons(usPort);
    
    	if(connect(sock, (PSOCKADDR)&sin, sizeof(SOCKADDR_IN)) == SOCKET_ERROR){
    		goto Failure;
    	}/*if*/
    
    	
    	/*Grab the right sized buffer needed*/
    	getBufSize = strlen("GET / HTTP/1.1\r\nHost: &#37;s\r\nConnection: close\r\n\r\n") + strlen(pszDest) + 2;
    
    	/*Allocate The Memory For The Buffer, NOTE: This seems to be where you corruption was, was the buffer big enough?*/
    	szGET = malloc(getBufSize * sizeof(szGET));
    	
    	/*Make sure we allocated properly*/
    	if(NULL == szGET){
    		goto Failure;
    	}/*if*/
    
    	/*Zero out the buffer*/
    	memset(szGET, '\0', getBufSize);
    
    	/*Format The Message To Send To The Host*/
    	wsprintf(szGET, "GET / HTTP/1.1\r\nHost: %s\r\nConnection: close\r\n\r\n", pszDest);
    
    	/*Send the message to the host*/
    	if(send(sock, szGET, strlen(szGET) + 1, 0) == SOCKET_ERROR){
    		goto Failure;
    	}/*if*/
    
    	do {
    		i = recv(sock, szBuf, 1023, 0);
    		
    		if(i < 1){
    			break;
    		}/*if*/
    
    		/*Make sure to null the buffer out*/
    		szBuf[i] = '\0';
    
    		p = strstr(szBuf, pszDesc);
    
    		if(NULL != p){
    			ip = p + strlen(pszDesc);
    
    			for(j = 0; j < iLen; ip++, j++){
    				if(isdigit(*ip) || *ip == '.'){
    					pszBuf[j] = *ip;
    				}/*if*/
    				else{
    					break;
    				}/*else*/
    			}/*for*/
    
    			pszBuf[j] = '\0';
    
    			/*Get out of the loop since we did what we need to do*/
    			break;
    		}/*if*/
    
    	} while(i > 0 && i != SOCKET_ERROR);
    
    	/*Clean up the goodies*/
    	closesocket(sock);
    	free(szGET);
    	WSACleanup();
    
    	return pszBuf;
    Failure:
    	closesocket(sock);
    	free(szGET);
    	WSACleanup();
    
    	return NULL;
    }
    
    int main()
    {
    	int szSize = 0;
    	char szStr[10];
    
    	/*Calc the size of the array. Note: This is only valid on arrays not pointer types!*/
    	szSize = sizeof(szStr) / sizeof(szStr[0]) - 1;
    
    	/*Message Box The Outputed Value*/
    	MessageBox(NULL, GetIP("whatismyip.com", "Your IP Is ", szStr, szSize, 80), "hi", MB_OK);
    
    	return 0;
    }
    Now I am a C++ guy, so you C guys yell at me if I blew anything, I don't think I did though.
    Last edited by prog-bman; 06-01-2007 at 03:18 PM.
    Woop?

  8. #8
    Madly in anger with you
    Join Date
    Nov 2005
    Posts
    211
    lol. I thought this message from the debugger seemed weird:

    Run-Time Check Failure #2 - Stack around the variable 'szGET' was corrupted.
    turns out that I was actually causing what I am trying to prevent here.

    Thanks all, fixed it with the advice given here and noticed that there was still a problem that was being caused by operator precedence.

    this:

    Code:
    for(j = 0; isdigit(*ip) || *ip == '.' && j < iLen; ip++, j++)
    should be:

    Code:
    for(j = 0; (isdigit(*ip) || *ip == '.') && j < iLen; ip++, j++)
    in order to be evaluated how I want it to be.

    all fixed now, printing proper results. thanks again.

    Intel Core 2 Quad Q6600 @ 2.40 GHz
    3072 MB PC2-5300 DDR2
    2 x 320 GB SATA (640 GB)
    NVIDIA GeForce 8400GS 256 MB PCI-E

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. C++ ini file reader problems
    By guitarist809 in forum C++ Programming
    Replies: 7
    Last Post: 09-04-2008, 06:02 AM
  2. We Got _DEBUG Errors
    By Tonto in forum Windows Programming
    Replies: 5
    Last Post: 12-22-2006, 04:45 PM
  3. Something is wrong with this menu...
    By DarkViper in forum Windows Programming
    Replies: 2
    Last Post: 12-14-2002, 10:06 PM
  4. Classes inheretance problem...
    By NANO in forum C++ Programming
    Replies: 12
    Last Post: 12-09-2002, 02:23 PM
  5. Warnings, warnings, warnings?
    By spentdome in forum C Programming
    Replies: 25
    Last Post: 05-27-2002, 06:49 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21