C Undefined symbol mid run of program

[noahbar]

I am running a C program that has some string split function calls.
Undefined symbol "strtok@FBSD_1.0" - During the string split, i get this call, i can clearly tell its the strtok function which is a tokenizer function. I have it in these files

#pragma once

#include <stdint.h> /* uint */
#include <string.h> /* char */
#include <stdarg.h> /* unknown number of arguments */
#include <stdio.h

/* user includes */
#include "characterArr.h"
/* user includes */
/* find a string in-between 2 other strings */
char* findBetween(char* left, char* right, char* fullString);
/* combine 2 strings into a new string */
char* combineString(int num, ...);
/* used to split the text file string so that only the token is stored */
void splitString(struct Arr* arr, char* str, char *delim);
/* replace all occurences of string with another */
void replaceString(char* original, char toReplace, char newChar);

/* user includes */
#include "str.h"
/* user includes */
void splitString(struct Arr* arr, char* str, char* delim) {
    if (!initialAlloc(arr)) {
        exit(1);
    }
    char* subStr = strtok(str, delim);

    while (subStr != NULL) {
        addToArr(arr, subStr);
     
        subStr = strtok(NULL, delim);
    }
}
char* findBetween(char* left, char* right, char* fullString) {
    int indexL = strstr(fullString, left) - fullString;
    int indexR = strstr(fullString, right) - fullString;
    char* parsed = calloc((indexR - indexL - strlen(left)), sizeof(char));
    strncpy(parsed, fullString + indexL + strlen(left), indexR - indexL - strlen(left));
    return parsed;
}

char* combineString(int num, ...) {

    char* finalStr;
    finalStr = calloc(600, sizeof(char));
    va_list vaList;
   
    /* initialize */
    va_start(vaList, num);
    for (int x = 0; x < num; x++) {
        char* str = va_arg(vaList, char*);
        strcat(finalStr, str);
    }
    va_end(vaList);
    return finalStr;
}

void replaceString(char* original, char toReplace, char newChar) {
    for (int i = 0; i < strlen(original); ++i) {
        if (original[i] == toReplace) {
            original[i] = newChar;
        }
    }    
}

It should be defined in string.h which i have included in the str.h header file shown above. What could be causing this?

 

[Alain De Vos]

Note you can post your code in a "[ CODE ]" "[ / CODE ] " block.

- Verify the functions receive null terminated strings.

- Try to link explicitly libc.a

 

[cracauer@]

This doesn't compile. Missing user include files.

 

[Andrey Lanin]

#include <stdio.h>

 

[zirias@]

Buffer overflow in combineStr() And in general wth is this code? At least as already mentioned, there's a lot just missing, so no way to reproduce anything and no way to help identifying the actual issue.

Edit: BTW, the comment of this vulnerable function is technically a lie:

/* combine 2 strings into a new string */
char* combineString(int num, ...);

 

[Alain De Vos]

"Write the result on the heap ?"

 

[zirias@]

Well, the "heap" is a technical construct unknown in the language C, but of course, almost all C implementations use a heap for "allocated objects".

Anyways, writing something there certainly isn't a problem in general. Writing a potentially unlimited sequence of bytes to an allocated object of size 600 (why 600? nice number? why not 666?) certainly is.

 

[Alain De Vos]

Note, for each calloc you must keep trac to free it or you have a memory leak.
In a sane program the total number of function calls to "calloc" equals the total number of function calls to "free".

 

[Paul Floyd]

Normally you would only get "undefined symbol" errors mid-run if you were trying to dlopen a library that is badly linked.

I doubt that is your case. It looks more like severe memory corruption, with the PLT getting trashed. (PLT - procedure linkage table, a table of function pointers that are resolved on the fly which allows shared libraries to be loaded to different addresses).

You need to post a fuller example, it'll be hard for us to guess what is causing your errors.

Finally, Valgrind will probably pinpoint the error straight away. Address Sanitizer or Memory Sanitizer may as well. And you should be able to find it just with lldb or gdb.

 

[noahbar]

Buffer overflow in combineStr() And in general wth is this code? At least as already mentioned, there's a lot just missing, so no way to reproduce anything and no way to help identifying the actual issue.

Edit: BTW, the comment of this vulnerable function is technically a lie:

/* combine 2 strings into a new string */
char* combineString(int num, ...);

Buffer overflow in combineStr() And in general wth is this code? At least as already mentioned, there's a lot just missing, so no way to reproduce anything and no way to help identifying the actual issue.

Edit: BTW, the comment of this vulnerable function is technically a lie:

/* combine 2 strings into a new string */
char* combineString(int num, ...);

i would imagine you're talking about the fact i just calloc a size of 600 then you could just input a string bigger? if not, go into more detail

 

[noahbar]

#include <stdio.h>

are you speaking about the random include? I was just trying to see if that somehow fixed the error which is why there is no comment bc theres no real use for it

 

[noahbar]

Note, for each calloc you must keep trac to free it or you have a memory leak.
In a sane program the total number of function calls to "calloc" equals the total number of function calls to "free".

is there a point in freeing memory that you calloc right before the progam ends? shouldn't that memory be freed anyways

 

[noahbar]

Well, the "heap" is a technical construct unknown in the language C, but of course, almost all C implementations use a heap for "allocated objects".

Anyways, writing something there certainly isn't a problem in general. Writing a potentially unlimited sequence of bytes to an allocated object of size 600 (why 600? nice number? why not 666?) certainly is.

if the function isn't using user input, would this matter since I know how many bytes can be stored?

 

[eternal_noob]

shouldn't that memory be freed anyways

C doesn't have a garbage collector. You are responsible to free memory you allocate on your own.

 

[Andrey Lanin]

are you speaking about the random include? I was just trying to see if that somehow fixed the error which is why there is no comment bc theres no real use for it

No, just about the closing '>'

 

[zirias@]

i would imagine you're talking about the fact i just calloc a size of 600 then you could just input a string bigger? if not, go into more detail

What do you mean, "if not"? This is a buffer overflow waiting to happen! With any possible consequences, from crashes, data corruption to easily exploitable security holes.

if the function isn't using user input, would this matter since I know how many bytes can be stored?

First, in a code review, I'd say "I don't believe you". Neither the name nor the documentation of that function tell anything about requiring the caller to pass at most 599 characters in total. Sure you could make this implicit and shift responsibility to the caller, but this would be an extremely weird requirement and then, why not just implement it correctly instead?

Furthermore, if you just needed a constant string, you could just write it in the source (or have it constructed by the preprocessor). So, something obviously depends on input. And input is not just (interactive) user input. You can't trust any input (from files, sockets, pipes, whatever), anything could be tampered with.

In a nutshell, never ever write such code at all. It's a security incident just waiting to happen.

 

[Alain De Vos]

A function should process in a sane way anyway it is called with arguments.
You could bail out if the length of the sum of the given strings is above 600 characters with an error message "sum lengths to long", but at least you would know what is happening and why.

 

[noahbar]

What do you mean, "if not"? This is a buffer overflow waiting to happen! With any possible consequences, from crashes, data corruption to easily exploitable security holes.


First, in a code review, I'd say "I don't believe you". Neither the name nor the documentation of that function tell anything about requiring the caller to pass at most 599 characters in total. Sure you could make this implicit and shift responsibility to the caller, but this would be an extremely weird requirement and then, why not just implement it correctly instead?

Furthermore, if you just needed a constant string, you could just write it in the source (or have it constructed by the preprocessor). So, something obviously depends on input. And input is not just (interactive) user input. You can't trust any input (from files, sockets, pipes, whatever), anything could be tampered with.

In a nutshell, never ever write such code at all. It's a security incident just waiting to happen.

if the function isn't using user input, would this matter since I know how many bytes can be stored?

i see, i used the term user input in a way that meant input that isn't from me. If the input is always from me then i can always control the overflow since i will know i cant pass more than x into finalStr, but you are right, i should just implement it correctly

 

[noahbar]

Note you can post your code in a "[ CODE ]" "[ / CODE ] " block.

- Verify the functions receive null terminated strings.

- Try to link explicitly libc.a

it seems that when using gcc, if i do /usr/lib/libc.a fixes the issue. Can you explain why i need to explicitly do this?

 

[Alain De Vos]

One of the functions compiles because you have a good "#include".
Still you need to link to the library which this function belongs in order to have a working executable.
libc contains many of the standard functions

 

[6502]

Is there any .cpp file extension in project?

 

[ralphbsz]

A few comments, not terribly organized.

You don't have to use calloc, it's just unneeded complexity. What calloc does is to multiply its two arguments, and then malloc that many bytes. You always use it with the second argument being sizeof(char), but that is guaranteed to be 1. So just use malloc.

Technically, you don't have to free memory right before the program exits. Whether memory is free'd when the program exits is a question that one doesn't have to think about, as there is no way to determine it. But not freeing memory is a bad habit, since it will (sooner or later) lead to memory leaks that break your programs.

Assuming that the user input is less than 600 bytes works, as long as the programmer and the user share that information. In your particular case, with it being the same person (you), and at nearly the same time, this might work. It is still a very bad practice. It is not particularly difficult to malloc exactly the correct amount of memory that is needed. When allocating memory, make sure you remember to include one extra byte for the trailing nul that ends the strings.

It is a bad practice to add more and more includes until the program compiles, without understanding why. It makes for messy code, which is more likely to break, and is difficult to maintain. For each include statement, you should be able to explain why it is there. It may be a good idea to comment each include, so a person reading and maintaining the code can see why it is there.

If your program has successfully compiled and loaded (that may be a single step, for example "c++ foo.C -o foo"), then you have found all the required link libraries. It is normally not necessary to explicitly specify linking against libc.

As your program is not using explicit dynamic linking, the error must be caused by a memory overwrite. Your code clearly has some bug. The problem with C-style string handling is that it is extremely brittle, and super easy to write broken code.

Here would be some design guidelines that may help you write more reliable code: (a) Always count the length of strings, and allocate exactly as much memory as necessary, no more and no less. (b) If you have allocated memory, keep track of how long it needs to be used. As soon as it is not needed any longer, free it. (c) When freeing memory, immediately null out the pointer to the freed memory, so you can't reuse the freed memory by mistake. (d) A generalization of the previous rule to memory that's on the stack (local variable): Limit their lifetime by using C++ blocks (opening and closing parentheses). Like that you can't use a variable after it is no longer valid. (e) Whenever possible, replace the normal str... functions (which rely on the terminating nul) with strn... functions (where you have to specify the length of the string, perhaps the maximum length). Those are much safer as they have fewer buffer overflow problems. (f) Replace strtok with strsep, which has a much saner call sequence, and doesn't modify the string itself.

 

[noahbar]

Is there any .cpp file extension in project?

no, pure C

 

[noahbar]

A few comments, not terribly organized.

You don't have to use calloc, it's just unneeded complexity. What calloc does is to multiply its two arguments, and then malloc that many bytes. You always use it with the second argument being sizeof(char), but that is guaranteed to be 1. So just use malloc.

Technically, you don't have to free memory right before the program exits. Whether memory is free'd when the program exits is a question that one doesn't have to think about, as there is no way to determine it. But not freeing memory is a bad habit, since it will (sooner or later) lead to memory leaks that break your programs.

Assuming that the user input is less than 600 bytes works, as long as the programmer and the user share that information. In your particular case, with it being the same person (you), and at nearly the same time, this might work. It is still a very bad practice. It is not particularly difficult to malloc exactly the correct amount of memory that is needed. When allocating memory, make sure you remember to include one extra byte for the trailing nul that ends the strings.

It is a bad practice to add more and more includes until the program compiles, without understanding why. It makes for messy code, which is more likely to break, and is difficult to maintain. For each include statement, you should be able to explain why it is there. It may be a good idea to comment each include, so a person reading and maintaining the code can see why it is there.

If your program has successfully compiled and loaded (that may be a single step, for example "c++ foo.C -o foo"), then you have found all the required link libraries. It is normally not necessary to explicitly specify linking against libc.

As your program is not using explicit dynamic linking, the error must be caused by a memory overwrite. Your code clearly has some bug. The problem with C-style string handling is that it is extremely brittle, and super easy to write broken code.

Here would be some design guidelines that may help you write more reliable code: (a) Always count the length of strings, and allocate exactly as much memory as necessary, no more and no less. (b) If you have allocated memory, keep track of how long it needs to be used. As soon as it is not needed any longer, free it. (c) When freeing memory, immediately null out the pointer to the freed memory, so you can't reuse the freed memory by mistake. (d) A generalization of the previous rule to memory that's on the stack (local variable): Limit their lifetime by using C++ blocks (opening and closing parentheses). Like that you can't use a variable after it is no longer valid. (e) Whenever possible, replace the normal str... functions (which rely on the terminating nul) with strn... functions (where you have to specify the length of the string, perhaps the maximum length). Those are much safer as they have fewer buffer overflow problems. (f) Replace strtok with strsep, which has a much saner call sequence, and doesn't modify the string itself.

going to fix everything mentioned, but still can not wrap my head around how a memory overwrite can cause a function to not be found? i was able to fix it by taking alain's advice

 

[6502]

#include <stdio.h

is it without > in real file ?