Francesco Totti
Francesco Totti

Reputation: 17

Why am i getting segmentation ? malloc() corrupted topsize

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


int main(int argc, char **argv, char **envp)
{

    for(int i=0; envp[i]!=NULL; i++)
        printf("%d.%s\n", i , envp[i]);
    printf("-----------------------------------\n");
    int maxim = -1;
    for(int i=0; i<1; i++)
    {
        int k=0;
        for(int j=0;envp[i][j]!=NULL; j++)
            k++;
        if(k>maxim)
            maxim=k;
    }
    char *string = (char*)malloc((sizeof(char)+1)*maxim);
    for(int i=0; envp[i]!=NULL; i++)
    {
        strcpy(string, envp[i]);
        for(int j=0;string[j]!=NULL;j++)
            if(string[j] == '=')
            {
                string[j]='\0';
                break;
            }
        if(i%2 == 0)
            setenv(string, "London", 1);
    }
    for(int i=0; envp[i]!=NULL; i++)
        printf("%d.%s\n", i , envp[i]);
    return 0;
}

I need to change the value of the enviorment var that is on an even pozition to "LONDON"

Upvotes: 0

Views: 84

Answers (1)

user9706
user9706

Reputation:

In the 2nd for loop you determine the length of the key=value string of the first environment variable. On my system that of the string "SHELL=/bin/bash" somaxism=15.

Then you allocate your string:

    char *string = malloc(2*maxim);

and then iterate through all environment variables and copy those to string. As my largest envp[i] is LS_COLORS which is 1519 bytes this will trigger a buffer overflow.

The minimal fix is to:

  1. Iterate through all of envp to find maxim.

  2. Size stringdifferently to avoid the potential issue of2 * maxium < strlen(key[i]) + sizeof('=') + strlen("London")`. It may not happen in practice but an attacker may manipulate the environment to make it so.

  3. Eliminate the != NULL mainly to address the warning when the lhs is of type char and rhs should have been != '\0'.

#define _POSIX_C_SOURCE 200112L
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main(int argc, char **argv, char **envp) {
    for(int i=0; envp[i]; i++)
        printf("%d.%s\n", i , envp[i]);
    printf("-----------------------------------\n");
    int maxim = -1;
    for(int i=0; envp[i]; i++) {
        int k=0;
        for(int j=0;envp[i][j]; j++)
            k++;
        if(k>maxim)
            maxim=k;
    }
    char *string = malloc(maxim + sizeof("London"));
    for(int i=0; envp[i]; i++) {
        strcpy(string, envp[i]);
        for(int j=0;string[j]; j++)
            if(string[j] == '=') {
                string[j] = '\0';
                break;
            }
        if(i%2 == 0)
            setenv(string, "London", 1);
    }
    for(int i=0; envp[i]; i++)
        printf("%d.%s\n", i , envp[i]);
    return 0;
}

You could simplify your code by:

  1. Write function to print the environment as you do it twice.
  2. Use strlen() 2nd for-loop instead of calculating it manually.
  3. In the 4th loop iterate over every 2nd element as it's the only ones being modified. Use strchr(envp[p], '=') to find the position of = then use strncpy() to only copy the key instead of the whole thing that you then have to post-process:
    for(int i=0; envp[i]; i+=2) {
        char *equal = strchr(envp[i], '=');
        if(!equal) continue; // key without = possible?
        strncpy(string, envp[i], equal - envp[i]);
        string[equal - envp[i]] = '\0';
        setenv(string, "London", 1);
    }

Upvotes: 1

Related Questions