Programming Forums > Projects > Show Off Your Open Source Projects

splinter9x · Jun 10th, 2006, 3:16 PM

Yup source would be nice...

Brent · Jun 10th, 2006, 4:14 PM

here's the source. I made a few changes to chiba to make it easier to use.

jayme · Jun 10th, 2006, 4:47 PM

That's not the source, that's the executable. I think you mixed up the files you were uploading. Unless you don't know the difference between source and executable. I don't think that's the problem here though.

hervens48 · Jun 10th, 2006, 4:53 PM

Awsome, even though i still cant chat, its still great
Can u please post the source files?
it would help me a lot

Brent · Jun 10th, 2006, 5:22 PM

oops...srry guys, i guess i forgot to attach the source, here it is. It should include chiba.cpp and chiba.h. the code is a bit messy, cuz I was just messing around with it and didnt really expect it to work. Ill post a cleaned up version sometime.

Mad_guy · Jun 10th, 2006, 5:54 PM

You have a whole bunch of flaws in your program that can cause the client to crash (and you can probably make the server crash too, although I haven't really looked into it.)

Here're some places that you need to examine:

(Toggle Plain Text)

case IDC_SEND:
{
 int len=GetWindowTextLength(GetDlgItem(hwnd,IDC_MAIN_EDIT));
 if(len > 0)
 {
   GetDlgItemText(hwnd,IDC_MAIN_EDIT,new_data,len+1);
   SetDlgItemText(hwnd,IDC_MAIN_EDIT,"");

First off, this causes an overflow on new_data. The problem here is that you're getting the length of the message, and then you're allowing that much data to be copied into new_data. The purpose of the last parameter of GetDlgItemText is to truncate data if it's too long, it relies entirely on the fact that your length calculation is correct. Meaning since new_data is 1024 bytes long (give or take, compilers and optimizations cause nuances in things like this,) if you type in 5000 characters, your program will copy it without problem because the last parameter of GetDlgItemText will say at that point "Let 5000+1 characters go through." What you need to change that to is something like this:

(Toggle Plain Text)

   GetDlgItemText(hwnd,IDC_MAIN_EDIT,new_data,sizeof(new_data)-3);

That way that will ensure that new_data is not overflowed.

Next:

(Toggle Plain Text)

strcpy(message,username);
strcat(message," : ");
strcat(message,new_data);

Now, you've truncated new_data and because new_data and message have the same length, you'd think that you couldn't overflow the message variable. Wrong, look at this code fragment:

(Toggle Plain Text)

case IDC_CHANGENAME:
{
 int len=GetWindowTextLength(GetDlgItem(hwnd,IDC_MAIN_EDIT));
 if(len > 0)
 {
  char *new_name;

  new_name=(char *)GlobalAlloc(GPTR,len+1);
  GetDlgItemText(hwnd,IDC_MAIN_EDIT,new_name,len+1);
				
  strcpy(username,"<");
  strcat(username,new_name);
  strcat(username,">");

  SetDlgItemText(hwnd,IDC_MAIN_EDIT,"");
 }
 else
 {
  MessageBox(NULL,"please select a username","msg",NULL);
 }			
}

There's more problems here: you allocate a buffer big enough to hold the name. Good, however, you then copy that onto the username variable which is only supposed to be 50 characters in length, and you copy it without length checking anyway. You can overflow this too, but let's say that you fix the first problem I pointed out (with new_data) and so only 1020 characters of data or so are copied into new_data. Okay, but then you strcat the 'username' variable onto it which can be (legitimately) 50 characters, so you're overflowing a 1070 character-or-so into a 1024 character buffer.

My advice to fix these bugs is that you start checking out your program logic a little more and start using functions like this:

A) snprintf
B) strncpy*

Hope this helped.

* strncpy is still not entirely safe. It will -not- terminate the destination buffer with a \0 character, so you could have non-null termination issues, here's an example:

(Toggle Plain Text)

char dest[256],finaldest[256];

strncpy(dest,user_input,sizeof(dest));
strcpy(finaldest,dest);

If user_input is like 500 characters long, dest won't be null terminated by strncpy. Meaning in the second strcpy, any adjacent buffers next to 'dest' could cause an overflow very easily since strcpy copies until a null is found. Here's a safe macro to ensure this won't happen:

(Toggle Plain Text)

/*ensures safe null termination on the
dest buffer if it is used in subsequent copies*/
#define safer_strncpy(dest,src,size) \
 strncpy(dest,src,sizeof(src)); dest[sizeof(dest)-1] = '\0';

jayme · Jun 10th, 2006, 6:05 PM

I've also been meaning to ask, what's with the name, "chiba"? I don't think that's an... appropriate name for a chat messenger. Maybe it's not the chiba I'm thinking of.

Brent · Jun 10th, 2006, 6:11 PM

thanks for the info

Brent · Jun 10th, 2006, 6:13 PM

idk.....i just randomly thought of a name for it, it just popped into my head.

b1g4L · Jun 10th, 2006, 6:14 PM

Did anyone ever say if this will or will not work over the Internet? j/w

Jun 10th, 2006, 3:16 PM	#21
splinter9x Hobbyist Programmer Join Date: Jun 2006 Posts: 137 Rep Power: 0	Yup source would be nice... __________________ Visit my Blog I support WINDOWS...

Jun 10th, 2006, 5:54 PM	#26
Mad_guy Hobbyist Programmer Join Date: Oct 2004 Location: Sandstorm, Techno Club Posts: 239 Rep Power: 4	You have a whole bunch of flaws in your program that can cause the client to crash (and you can probably make the server crash too, although I haven't really looked into it.) Here're some places that you need to examine: (Toggle Plain Text) case IDC_SEND: { int len=GetWindowTextLength(GetDlgItem(hwnd,IDC_MAIN_EDIT)); if(len > 0) { GetDlgItemText(hwnd,IDC_MAIN_EDIT,new_data,len+1); SetDlgItemText(hwnd,IDC_MAIN_EDIT,""); case IDC_SEND: { int len=GetWindowTextLength(GetDlgItem(hwnd,IDC_MAIN_EDIT)); if(len > 0) { GetDlgItemText(hwnd,IDC_MAIN_EDIT,new_data,len+1); SetDlgItemText(hwnd,IDC_MAIN_EDIT,""); First off, this causes an overflow on new_data. The problem here is that you're getting the length of the message, and then you're allowing that much data to be copied into new_data. The purpose of the last parameter of GetDlgItemText is to truncate data if it's too long, it relies entirely on the fact that your length calculation is correct. Meaning since new_data is 1024 bytes long (give or take, compilers and optimizations cause nuances in things like this,) if you type in 5000 characters, your program will copy it without problem because the last parameter of GetDlgItemText will say at that point "Let 5000+1 characters go through." What you need to change that to is something like this: (Toggle Plain Text) GetDlgItemText(hwnd,IDC_MAIN_EDIT,new_data,sizeof(new_data)-3); GetDlgItemText(hwnd,IDC_MAIN_EDIT,new_data,sizeof(new_data)-3); That way that will ensure that new_data is not overflowed. Next: (Toggle Plain Text) strcpy(message,username); strcat(message," : "); strcat(message,new_data); strcpy(message,username); strcat(message," : "); strcat(message,new_data); Now, you've truncated new_data and because new_data and message have the same length, you'd think that you couldn't overflow the message variable. Wrong, look at this code fragment: (Toggle Plain Text) case IDC_CHANGENAME: { int len=GetWindowTextLength(GetDlgItem(hwnd,IDC_MAIN_EDIT)); if(len > 0) { char new_name; new_name=(char )GlobalAlloc(GPTR,len+1); GetDlgItemText(hwnd,IDC_MAIN_EDIT,new_name,len+1); strcpy(username,"<"); strcat(username,new_name); strcat(username,">"); SetDlgItemText(hwnd,IDC_MAIN_EDIT,""); } else { MessageBox(NULL,"please select a username","msg",NULL); } } case IDC_CHANGENAME: { int len=GetWindowTextLength(GetDlgItem(hwnd,IDC_MAIN_EDIT)); if(len > 0) { char new_name; new_name=(char )GlobalAlloc(GPTR,len+1); GetDlgItemText(hwnd,IDC_MAIN_EDIT,new_name,len+1); strcpy(username,"<"); strcat(username,new_name); strcat(username,">"); SetDlgItemText(hwnd,IDC_MAIN_EDIT,""); } else { MessageBox(NULL,"please select a username","msg",NULL); } } There's more problems here: you allocate a buffer big enough to hold the name. Good, however, you then copy that onto the username variable which is only supposed to be 50 characters in length, and you copy it without length checking anyway. You can overflow this too, but let's say that you fix the first problem I pointed out (with new_data) and so only 1020 characters of data or so are copied into new_data. Okay, but then you strcat the 'username' variable onto it which can be (legitimately) 50 characters, so you're overflowing a 1070 character-or-so into a 1024 character buffer. My advice to fix these bugs is that you start checking out your program logic a little more and start using functions like this: A) snprintf B) strncpy* Hope this helped. * strncpy is still not entirely safe. It will -not- terminate the destination buffer with a \0 character, so you could have non-null termination issues, here's an example: (Toggle Plain Text) char dest[256],finaldest[256]; strncpy(dest,user_input,sizeof(dest)); strcpy(finaldest,dest); char dest[256],finaldest[256]; strncpy(dest,user_input,sizeof(dest)); strcpy(finaldest,dest); If user_input is like 500 characters long, dest won't be null terminated by strncpy. Meaning in the second strcpy, any adjacent buffers next to 'dest' could cause an overflow very easily since strcpy copies until a null is found. Here's a safe macro to ensure this won't happen: (Toggle Plain Text) /ensures safe null termination on the dest buffer if it is used in subsequent copies/ #define safer_strncpy(dest,src,size) \ strncpy(dest,src,sizeof(src)); dest[sizeof(dest)-1] = '\0'; /ensures safe null termination on the dest buffer if it is used in subsequent copies/ #define safer_strncpy(dest,src,size) \ strncpy(dest,src,sizeof(src)); dest[sizeof(dest)-1] = '\0'; __________________ os: mac os 10.5.4 revision control: git editor: emacs site

Thread Tools
Show Printable Version Email this Page
Display Modes
Linear Mode Switch to Hybrid Mode Switch to Threaded Mode

Jun 10th, 2006, 4:53 PM	#24
hervens48 Programmer Join Date: Apr 2006 Location: Montreal, Canada Posts: 93 Rep Power: 3	Awsome, even though i still cant chat, its still great Can u please post the source files? it would help me a lot

Jun 10th, 2006, 6:11 PM	#28
Brent Highly Adaptive Penguin Join Date: May 2005 Location: United States Posts: 251 Rep Power: 4	thanks for the info

Jun 10th, 2006, 6:13 PM	#29
Brent Highly Adaptive Penguin Join Date: May 2005 Location: United States Posts: 251 Rep Power: 4	idk.....i just randomly thought of a name for it, it just popped into my head.

Jun 10th, 2006, 6:14 PM	#30
b1g4L Programmer Join Date: Dec 2005 Location: SC Posts: 38 Rep Power: 0	Did anyone ever say if this will or will not work over the Internet? j/w

Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)

Programming Forums > Projects > Show Off Your Open Source Projects

Chiba Instant Messenger