Undefined Behavior

[bbaale]

Why would this code cause undefined behavior?

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

#define TRUE 1
#define FALSE 0
#define MAX_PASSWORD 11
int loop;
unsigned char rx;
int _strlen(s)
unsigned char s[];
{
int i;

i=0;
while(s[i]!='\0')
++i;
return i;
}

int encrypt (s, x, len)
unsigned char *s ;
unsigned char x;
int len;
{

int i,ok;
unsigned char seed_1,seed_2;

seed_1 = rx;
seed_2 = rx + 31;
ok = TRUE;
for(i = 0 ; i < len ; i++)
{
seed_1 -= i;
seed_2 += i;
if(i%2 != 0)
s[i] -= seed_1;
else
s[i] += seed_2;
if(s[i] == '&' || s[i]== '\0' || s[i] == '\n')
ok = FALSE;
}
if (x == '&')
ok = FALSE;
return ok;
}

int decrypt (s, len)
unsigned char *s;
int len ;

{
unsigned char seed_1, seed_2;
int i;

seed_1 = rx;
seed_2 = rx+31;
for (i = 0; i < len ; i++)
{
seed_1 -= i;
seed_2 +=i;
if (i % 2 != 0)
s[i] += seed_1;
else
s[i] -= seed_2;
}
return TRUE;
}

int encrypt_string (s)
unsigned char *s ;
{

unsigned char x;
int c, len;

x=0;
c=FALSE;
len= _strlen(s);

while( c!= FALSE || loop>0)
{
x++;
c=encrypt(s, x, len);
loop--;
}
s[len++] = x;
s[len++] = rx;
s[len]= '\0';

return TRUE;
}
int decrypt_string( s)
unsigned char *s;
{

unsigned char x,y ;
int len;

len = _strlen(s)-1;

rx= s[len];
len--;
x=s[len];
for(y = 0; y < x ; y++)
decrypt(s, len);
s[len]= '\0';

return TRUE;
}
int main() {

unsigned char *s;
unsigned char endchar;
unsigned int y;
int len;
s=(unsigned char *) calloc( MAX_PASSWORD, sizeof(unsigned char) );
if(s==NULL){
printf("Could not allocate Memory");
return 0;
}

fgets(s,MAX_PAS SWORD,stdin);
len=_strlen(s);

endchar =s[len-1];

if(endchar=='\r ' || endchar=='\n')
s[len-1]='\0';

rx=strlen(s) ;
loop=1;

y=encrypt_strin g(s);

if(y==TRUE)
printf("\n\nEnc rypted String :%s ######%d\n\n",s ,_strlen(s));

y=decrypt_strin g(s);
if(y==TRUE)
printf("Decrypt ed String: %s %d\n",s,_strlen (s));

if(*s=!NULL){
free(s);
s=NULL;
}
else
printf("Could not deallocate memory\n");
return 0;

}

[Richard Heathfield]

bb****@gmail.co m said:

Why would this code cause undefined behavior?

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

#define TRUE 1
#define FALSE 0
#define MAX_PASSWORD 11
int loop;
unsigned char rx;
int _strlen(s)

Invading implementation namespace.

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: rjh at the above domain, - www.

[Richard Tobin]

In article <11************ **********@r35g 2000prh.googleg roups.com>,
<bb****@gmail.c omwrote:

>#define MAX_PASSWORD 11

s=(unsigned char *) calloc( MAX_PASSWORD, sizeof(unsigned char) );
if(s==NULL){
printf("Could not allocate Memory");
return 0;
}

fgets(s,MAX_PAS SWORD,stdin);
len=_strlen(s);

What's this _strlen()? Apart from anything else, that's a reserved
name.

>
endchar =s[len-1];

if(endchar=='\r ' || endchar=='\n')
s[len-1]='\0';

So at this point s points to a string up to 11 bytes long (including
the nul at the end) in a buffer 11 bytes long.

y=encrypt_strin g(s);

>int encrypt_string (s)

....

s[len++] = x;
s[len++] = rx;
s[len]= '\0';

Woops! You're adding things on to the end of s, and overflowing the
buffer.

You should be able to find mistakes like this yourself, using a
memory checking tool such as valgrind.

-- Richard
--
"Considerat ion shall be given to the need for as many as 32 characters
in some alphabets" - X3.4, 1963.

[CBFalconer]

bb****@gmail.co m wrote:

>
Why would this code cause undefined behavior?

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

#define TRUE 1
#define FALSE 0
#define MAX_PASSWORD 11
int loop;
unsigned char rx;

int _strlen(s)
unsigned char s[];
{

Because right here you have used a label reserved for the
implementation. Don't do that.

--
<http://www.cs.auckland .ac.nz/~pgut001/pubs/vista_cost.txt>
<http://www.securityfoc us.com/columnists/423>
<http://www.aaxnet.com/editor/edit043.html>
cbfalconer at maineline.net

--
Posted via a free Usenet account from http://www.teranews.com

[Martin Ambuhl]

bb****@gmail.co m wrote:

Why would this code cause undefined behavior?

It was hard to resist rewriting this mess. Having resisted, I still
refuse to quote it all, except to note that

if(*s=!NULL){
free(s);
s=NULL;
}

probably is not what you meant.
if (*s = !NULL) ?* use some whitespace, for pete's sake */
does not mean the same thing as what you probably meant,
if (*s)

[Gregor H.]

On 25 Apr 2007 05:50:31 -0700, bb****@gmail.co m wrote:

>
if(*s=!NULL){
free(s);
s=NULL;
}
else
printf("Could not deallocate memory\n");

Should read
if (s != NULL)
{
free(s);
s = NULL;
}
else
printf("Could not deallocate memory\n");
I guess.
G.

--

E-mail: info<at>simple-line<Punkt>de

[Christopher Benson-Manica]

Martin Ambuhl <ma*****@earthl ink.netwrote:

bb****@gmail.co m wrote:

if(*s=!NULL){
free(s);
s=NULL;
}

you probably meant,
if (*s)

I wouldn't think so; I see no reason why a dynamically allocated
string shouldn't be freed simply because its length is 0.

--
C. Benson Manica | I *should* know what I'm talking about - if I
cbmanica(at)gma il.com | don't, I need to know. Flames welcome.

[Martin Ambuhl]

Christopher Benson-Manica wrote:

Martin Ambuhl <ma*****@earthl ink.netwrote:

>bb****@gmail.co m wrote:

>> if(*s=!NULL){
free(s);
s=NULL;
}

>you probably meant,
if (*s)

I wouldn't think so; I see no reason why a dynamically allocated
string shouldn't be freed simply because its length is 0.

You're right. But guessing what the OP meant by his assignment
statement is up in the air. Any guess is likely to be wrong.

[Gregor H.]

On Thu, 26 Apr 2007 00:29:11 -0400, Martin Ambuhl
<ma*****@earthl ink.netwrote:

>>>>
if(*s=!NULL){
free(s);
s=NULL;
}

>>you probably meant,
if (*s)

I wouldn't think so; I see no reason why a dynamically allocated
string shouldn't be freed simply because its length is 0.

You're right. But guessing what the OP meant by his assignment
statement is up in the air. Any guess is likely to be wrong.

Not so!

It should read
if (s != NULL)
{
free(s);
s = NULL;
}
else
printf("Could not deallocate memory\n");
I guess. (Such a construction -without the else-path- does make sense
_if_ it is possible that the allocated memory might have been freed
already at an earlier time. I once used such a construction, IIRC.)
G.

--

E-mail: info<at>simple-line<Punkt>de

[Joachim Schmitz]

"Gregor H." <nomail@invalid schrieb im Newsbeitrag
news:us******** *************** *********@4ax.c om...

On Thu, 26 Apr 2007 00:29:11 -0400, Martin Ambuhl
<ma*****@earthl ink.netwrote:

>>>>>
if(*s=!NULL){
free(s);
s=NULL;
}

you probably meant,
if (*s)

I wouldn't think so; I see no reason why a dynamically allocated
string shouldn't be freed simply because its length is 0.

You're right. But guessing what the OP meant by his assignment
statement is up in the air. Any guess is likely to be wrong.

Not so!

It should read
if (s != NULL)
{
free(s);
s = NULL;
}
else
printf("Could not deallocate memory\n");
I guess. (Such a construction -without the else-path- does make sense
_if_ it is possible that the allocated memory might have been freed
already at an earlier time. I once used such a construction, IIRC.)

You can feed free() with anything that got returned by malloc(), calloc() or
realloc() which in turn can return NULL, so you can safely call free(NULL)
unconditionally and safe that exta code.

free(s),s = NULL;

The mesages "Could not deallocate memory" is a) not neccessary (as you
haven't got any memory to free in the first place) and b) if at all should
go to stderr, shouldn't it?

Bye, Jojo