Code causes segfault on exit

[James Leddy]

For some reason, I get a segmentation fault when I exit this program I made.
Most of the time when I have seen this it has occured in the middle of the
program and the program terminates. However, in this instance, the program
gets a segmentation fault on exit.

I already posted this question, but people were asking for some code, so
here it is:


#define DWORD unsigned long
/* called from main() */
int extract(struct tm *date) {
FILE *journ;
DWORD full_date;
DWORD tmp_date;
DWORD index_size;
DWORD index_offset;
DWORD sear_size;
DWORD *index;
DWORD *bsear;
DWORD *esear;
DWORD *psear;
DWORD header[2];
size_t pwd_len;
int extra;
char *raw_buf;
char *clear_buf;
char **pwd;
char *zerout;
char chk[16];

full_date = ((date->tm_year) << 16) | ((date->tm_mon) << 8)
| (date->tm_mday);
journ = fopen(FNAME, "r");
if (fread(chk, 1, 16, journ) == 0)
return FILE_ERROR;
if (memcmp(chk, "EncryptedJournal", 16))
return FILE_ERROR;
fread(&index_offset, sizeof(index_offset), 1, journ);
fseek(journ, index_offset, SEEK_SET);
fread(&index_size, sizeof(index_size), 1, journ);
if ((index = malloc((index_size) * sizeof(*index))) == NULL)
return ERROR;
fread(index, sizeof(*index), index_size, journ);

bsear = index;
esear = index + (index_size - 1);
if (index_size == 1) {
psear = index;
fseek(journ, *psear, SEEK_SET);
fread(&tmp_date, sizeof(tmp_date), 1, journ);
}
for (sear_size = index_size; sear_size > 2;
sear_size = (esear - bsear) + 1) {

if (tmp_date < full_date)
psear = bsear + (sear_size / 2);
else
psear = esear - (sear_size/2);

fseek(journ, *psear, SEEK_SET);
fread(&tmp_date, sizeof(tmp_date), 1, journ);
if (tmp_date == full_date) {
break;
} else if (tmp_date < full_date) {
bsear = psear;
} else if (tmp_date > full_date) {
esear = psear;
}
}
if (sear_size == 2) {
fseek(journ, *bsear, SEEK_SET);
fread(&tmp_date, sizeof(tmp_date), 1, journ);
if (tmp_date != full_date) {
fseek(journ, *esear, SEEK_SET);
fread(&tmp_date, sizeof(tmp_date), 1, journ);
}
}
if (tmp_date != full_date)
return DATE_ERROR;
fread(header, sizeof(*header), 2, journ);
if ((raw_buf = malloc(header[0])) == NULL)
return ERROR;
fread(raw_buf, sizeof(*raw_buf), header[0], journ);

if ((pwd = malloc(1)) == NULL)
return ERROR;
*pwd = NULL;
if ((pwd_len = get_pwd(pwd)) == ERROR) {
printf("Passwords don't match\nSorry =(\n");
free(*pwd);
free(pwd);
return ERROR;
}
if (initialize_blowfish(*pwd, pwd_len) == ERROR) {
zerout = calloc(pwd_len + 1, 1);
memcpy(*pwd, zerout, pwd_len + 1);
free(zerout);
free(*pwd);
free(pwd);
return ERROR;
}
zerout = calloc(pwd_len + 1, 1);
memcpy(*pwd, zerout, pwd_len + 1);
free(zerout);
free(*pwd);
free(pwd);

extra = decipher(raw_buf, header[0]);
header[0] -= extra;
if ((raw_buf = realloc(raw_buf, header[0])) == NULL)
return ERROR;
if ((clear_buf = malloc(header[1])) == NULL)
return ERROR;
uncompress(clear_buf, (header + 1), raw_buf, header[0]);
fwrite(clear_buf, 1, header[1], stdout);
free(raw_buf);
free(clear_buf);
free(index);
return SUCCESS;
}

/* called from main() */
int conv_str_date(struct tm *result, char *date)
{
char *number;
long int tmp;

//month
if ((number = strtok(date, "/")) == NULL)
return ERROR;
tmp = strtol(number, NULL, 10);
if ((tmp < 0) || (tmp > 12))
return ERROR;
result->tm_mon = tmp - 1;
//day
if ((number = strtok(NULL, "/")) == NULL)
return ERROR;
tmp = strtol(number, NULL, 10);
if ((tmp < 1) || (tmp > 31))
return ERROR;
result->tm_mday = tmp;
//year
if ((number = strtok(NULL, "\0")) == NULL)
return ERROR;
tmp = strtol(number, NULL, 10);
tmp-=1900;
if (tmp < 0)
return ERROR;
result->tm_year = tmp;
return 1;
}

/* called from extract() */
int get_pwd(char **pwd)
{
struct termios old, new;
char **check;
char *s;
char *zerout;
int i;
size_t scheck = 0;
size_t spwd = 0;

if ((check = malloc(1)) == NULL)
return ERROR;
*check = NULL;
if (tcgetattr(fileno(stdin), &old) != 0)
return ERROR;
new = old;
new.c_lflag &= ~ECHO;
if (tcsetattr(fileno(stdin), TCSAFLUSH, &new) != 0)
return ERROR;
for (i = 0; i < 3; i++) {
printf("Password: ");
getline(pwd, &spwd, stdin);
printf("\nAgain: ");
getline(check, &scheck, stdin);
if (!strcmp(*pwd, *check))
break;
printf("\nPasswords don't match\n");
}
tcsetattr(fileno(stdin), TCSAFLUSH, &old);
if ((zerout = calloc(scheck, 1)) == NULL)
return ERROR;
printf("\n");
memcpy(*check, zerout, scheck);
scheck = 0;
free(*check);
free(check);
free(zerout);
if (i == 3) {
if ((zerout = calloc(spwd, 1)) == NULL)
return ERROR;
memcpy(*pwd, zerout, spwd);
return ERROR;
free(zerout);
}
assert((s = strchr(*pwd, (int)'\n')) != NULL);
*s = 0; //we don't want the newlinet in our password!!
return strlen(*pwd);
}

void decipher_dword(void *al, void *ar)
{
DWORD *temp, *xl, *xr;
int i;

xl = al;
xr = ar;
for (i = N + 1; i > 1; i--) {
*xl = *xl ^ P;
*xr = f(xl) ^ *xr;
SWAP(*xl, *xr, *temp);
}

SWAP(*xl, *xr, *temp);

*xr = *xr ^ P[1];
*xl = *xl ^ P[0];
}

int decipher(char *buf, size_t size)
{
div_t fix;
int i;
char *pbuf;
DWORD value;

fix = div(size, 8);
assert (fix.rem == 0);
for (i = 0; i < fix.quot; i++) {
pbuf = buf + (i * 8);
decipher_dword(pbuf, pbuf + 4);
}
if ((value = buf[size -1]) < 8)
return value;
return 0;
}

I know it's a lot, that is why I did not want to post it in the first place.

Thanks again,
James Leddy

 

[Nudge]

#define DWORD unsigned long

Isn't a typedef more appropriate?

 

[Eric Sosman]

For some reason, I get a segmentation fault when I exit this program I made.
Most of the time when I have seen this it has occured in the middle of the
program and the program terminates. However, in this instance, the program
gets a segmentation fault on exit.

I already posted this question, but people were asking for some code, so
here it is: [heavily snipped]


#define DWORD unsigned long
/* called from main() */
int extract(struct tm *date) {
[...]
journ = fopen(FNAME, "r");

You probably want "rb" there, since you appear to be
reading binary data as opposed to text.

if ((pwd = malloc(1)) == NULL)
return ERROR;
*pwd = NULL;

Here's one problem, at any rate. `pwd' is a `char**',
and unless sizeof(char*) happens to be 1 on your system
you're storing more data than you've allocated space for.
It's likely that you'll get away with this error, though,
because malloc() implementations usually round up the
requested size to a convenient multiple of some "atom"
size, and that'll probably be enough to hold the extra
data. Risky, though, and you should fix it.

if (initialize_blowfish(*pwd, pwd_len) == ERROR) {
zerout = calloc(pwd_len + 1, 1);
memcpy(*pwd, zerout, pwd_len + 1);
free(zerout);

Why fool around with this `zerout' area, when a simple
`memset(*pwd, 0, pwd_len + 1)' does the same thing with
less bother and without risking calloc() failure?

uncompress(clear_buf, (header + 1), raw_buf, header[0]);

You haven't shown us what uncompress() looks like, but that
second argument looks peculiar. Are you sure you didn't mean
`*(header + 1)' or more simply `header[1]'? If it's actually
an output from uncompress() it may be right, but check it.

/* called from extract() */
int get_pwd(char **pwd)
{
[...]
if ((check = malloc(1)) == NULL)
return ERROR;
*check = NULL;

This is the same error you made with `pwd' in extract().

If I were trying to debug this, my next step would be
to sprinkle a bunch of

fprintf (stderr, "Made it to line %d\n", __LINE__);

statements at strategic places, particularly in the latter
stages of the program where the problem appears (which is not
necessarily where it actually occurs). This technique is
surprisingly effective in helping narrow in on the actual
point of failure, but beware: if the problem arises from a
"wild pointer" stomping on unpredictable memory locations,
adding a few function calls may move things around enough
that the problem disappears or changes its manifestation.
Still, it remains a useful, if non-sexy, technique.

Another thing I'd do is check for failure of those I/O
functions. It really makes me nervous to see all that
blind trust in a number that may or may not have actually
been read ...

 

[Christian Bau]

Isn't a typedef more appropriate?

DWORD (abbreviation for "double word") is not appropriate, whether you
use a #define or a typedef. On the majority of current computers,
"unsigned long" is a word and not a double word; on a small but growing
number of computers "unsigned long" is a half word.

 

[Mark Gordon]

For some reason, I get a segmentation fault when I exit this program I
made. Most of the time when I have seen this it has occured in the
middle of the program and the program terminates. However, in this
instance, the program gets a segmentation fault on exit.

I already posted this question, but people were asking for some code,
so here it is:

You missed at least the following includes. However, even with them
added this does not come anywhere near compiling and therefor is NOT
your source code.

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

#define DWORD unsigned long

You should use a typedef rather than a #define. Look in your C reference
for the syntax.

/* called from main() */
int extract(struct tm *date) {
FILE *journ;
DWORD full_date;
DWORD tmp_date;
DWORD index_size;
DWORD index_offset;
DWORD sear_size;
DWORD *index;
DWORD *bsear;
DWORD *esear;
DWORD *psear;
DWORD header[2];
size_t pwd_len;
int extra;
char *raw_buf;
char *clear_buf;
char **pwd;
char *zerout;
char chk[16];

full_date = ((date->tm_year) << 16) | ((date->tm_mon) << 8)
| (date->tm_mday);
journ = fopen(FNAME, "r");

t.c:30: error: `FNAME' undeclared (first use in this function)

The first of many.

I know it's a lot, that is why I did not want to post it in the first
place.

Then try deleting bits to cut it down to the minimum COMPILABLE example
that exhibits your problem. Even if I took guesses at how identifiers
should be declared, you have not included a definition of main so I
don't know how you use the functions.

Amongst the things you should delete (or replace with suitable stubs)
are all the OS dependant things such as getline (a GNU extension),
tcsetattr which is a UNIX function etc.

I also suggest you look up in your compiler manual on how to turn up the
warning level then deal with all the many warning you will get.

I also suggest you look up the definition of malloc and then check your
usage since the following is also an error
char **pwd;
if ((pwd = malloc(1)) == NULL)
return ERROR;
*pwd = NULL;

Finally I suggest you go back to something far simpler such as working
your way through the examples in a GOO C text book such as K&R2.

 

[Irrwahn Grausewitz]

For some reason, I get a segmentation fault when I exit this program I made.
Most of the time when I have seen this it has occured in the middle of the
program and the program terminates. However, in this instance, the program
gets a segmentation fault on exit.

I already posted this question, but people were asking for some code, so
here it is:

Fine, but it would heve been better to post your code in the thread you
already started, after cutting it down to a minimal version exhibiting
your problem.

#define DWORD unsigned long

typedef unsigned long DWORD;

char **pwd;

if ((pwd = malloc(1)) == NULL)

Ouch! One byte is most certainly insufficient to hold a char *.
All bets off. No further checking applied.

I know it's a lot, that is why I did not want to post it in the first place.

Why didn't you reduce it?

Regards

 

[James Leddy]

Why didn't you reduce it?

I would have if I could have tracked down the runtime error to a line or
function. But because the error occured upon exit, I had no idea what
function the malicious code was in. So, I included all the code that was
called during the course of running this program in the erroneous case.

 

[Irrwahn Grausewitz]

I would have if I could have tracked down the runtime error to a line or
function. But because the error occured upon exit, I had no idea what
function the malicious code was in. So, I included all the code that was
called during the course of running this program in the erroneous case.

Well, you could replace functions with dummy versions (one at a time)
and check whether the error goes away.

Furthermore, if you'd pumped up the warning level of your compiler, it'd
given you several hints to flaws in your code.

Finally, to track down errors, the use of a debugger has been reported
to be somewhat helpful. ;-)

Regards