cmemory

Assignment overlaps memory


I have a function that takes two hex strings, XORs them and will create a string with the result(once I iron out the bug in it).

int myXORFunction(char *firstInputString, char *secondInputString){
    //This function takes two hex strings of equal length, XORs each pair of bytes and returns the resulting string.
    char firstReconstructedByte[2] = {0};
    char secondReconstructedByte[2] = {0};
    char buffer[1];
    int firstResult, secondResult, xorResult;

    if (strlen(firstInputString) != strlen(secondInputString)){
        printf("String lengths don't match exiting...");
        return 1;
    }else{
        //printf("More code coming soon!\n");
        for(int index=0; index != strlen(firstInputString); index = index +2){
            firstReconstructedByte[0] = firstInputString[index];
            firstReconstructedByte[1] = firstInputString[index+1];
            //firstReconstructedByte[2] = 0;
            firstResult  = strtol(firstReconstructedByte, 0, 16);

            secondReconstructedByte[0] = secondInputString[index];
            secondReconstructedByte[1] = secondInputString[index+1];
            //secondReconstructedByte[2] = 0;
            secondResult  = strtol(secondReconstructedByte, 0, 16);

            xorResult = firstResult^secondResult;

            //printf("First operand is: %s\n", firstReconstructedByte);
            //printf("Second operand is: %s\n", secondReconstructedByte);

            printf("Result of XORing %s and %s is: %d\n", firstReconstructedByte, secondReconstructedByte, xorResult);
        }

        return 0;
    }
}

It works fairly well. However, at the final print statement, it doesn't ouput anything for the firstReconstructedByte variable. The offending code is the linesecondReconstructedByte[2] = 0; which puts a null byte into the memory pointed to by firstReconstructedByte.

Is this a compiler bug? Is there a compiler directive I can use to byte-align firstReconstructedByte and secondReconstructedByte so that working with one doesn't trespass the memory used by the other?

A screenshot before secondReconstructedByte[2] = 0; runs is attached below: Before

A screenshot after it runs is attached below: After

I'm using gcc 14.2 on Linux. My value for firstInputString is 1c0111001f010100061a024b53535009181c and secondInputString is 686974207468652062756c6c277320657965.

Edit 1: Following the advice I've recieved, I'm no longer assigning the null byte, or, accessing the 3rd element of my arrays any more. However, the output of secondReconstructedByte now gets corrupted.


Solution

  • Code has at least these problems:

    Misuse of strtol()

    With strtol(firstReconstructedByte, 0, 16), the function expects a pointer to a string*1 as the first argument. The char array firstReconstructedByte[] does not certainly contain a string as it may lack a null character.

    Misuse of "%s"

    printf("...%s...", firstReconstructedByte has "%s" and so expects to receive a pointer to a string. The char array firstReconstructedByte[] does not certainly contain a string as it may lack a null character.

    Make character array large enough for a string of length 2

    Making the array large enough should solve most issues.

    To store a string of string length 2, the character array needs to be at least size 3 to also store the null character '\0'.

    //char firstReconstructedByte[2] = {0};
    //char secondReconstructedByte[2] = {0};
    char firstReconstructedByte[2+1] = {0};
    char secondReconstructedByte[2+1] = {0};
    

    Odd length string?

    if (strlen(firstInputString) != strlen(secondInputString)){ is a good test, yet code may also want to insure the common length is even.

    Null character termination

    Uncommenting //firstReconstructedByte[2] = 0; after increasing array size is OK and a preferable style, yet not needed as the array initializations already do this.

    Idea: Use hex output

    Since input is interpreted as hexadecimal characters, using "0x%X" to print the xorResult likely more informative that "%d".

    Consider using unsigned firstResult, secondResult, xorResult;.

    Weakness: no conversion check

    With strtol(firstReconstructedByte, 0, 16);, strtol() receives 0 for the end pointer address. This prevents testing if the conversion succeeded.

    OP does say "function that takes two hex strings" so a failed conversion is not excepted.

    User input is evil. I prefer a test.

    Perhaps something like:

        char *endptr;
        firstReconstructedByte = strtol(firstReconstructedByte, &endptr, 16);
        if (endptr != firstReconstructedByte + 2) {
          // Conversion did not use 2 characters.
          TBD code to print error
        }
    

    Potential hole

    "and will create a string with the result" could be a problem if the formed string is only 1 byte per xorResult. When xorResult == 0, as 1 byte, is a potential troublesome early null character.

    Good luck.


    *1
    C library definition of string

    A string is a contiguous sequence of characters terminated by and including the first null character.