-4

Why does my output contain extra characters? Why is only the first line of every file via notepad++ being encrypted and not the entire file? Happy coding! P.S I have the Second Edition of C programming language by Kernighan and Ritchie

EDIT: This code is my code after I fixed it, the question's has been answered. Thank you guys!

Here is my source NEW code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define getchar() getc(stdin)
#define putchar() putc((c),stdout)
#define XOR_BYTE 0x9E

char * xorBuffer(char *buffer, long bufferSize){

    int i;
    for(i = 0;i <= bufferSize;i++){
        buffer[i] ^= XOR_BYTE;
    }
    return buffer;
}

int xorFile(char *fileIn, char * fileOut){

    FILE *fpi, *fpo;
    char *fileBuffer = NULL;

    fpi = fopen(fileIn,"rb");
    fpo = fopen(fileOut,"wb");

    if(NULL == fpi){
        printf("Error opening input file %s: %s\n", fileIn, strerror(errno));
        return 1;
    }
    if(NULL == fpo){
        printf("Error opening output file %s: %s\n", fileOut, strerror(errno));
        return 2;
    }

    fseek(fpi,0L,SEEK_END);
    long fileSize = ftell(fpi); 
    fileBuffer = malloc(sizeof(char)* (fileSize + 1));  
    fseek(fpi,0L,SEEK_SET);     
    size_t length = fread(fileBuffer, sizeof(char), fileSize,fpi);      
    fileBuffer[length];
    fileBuffer = (char *)xorBuffer(fileBuffer,fileSize);    
    int c;  
    for(c = 0;c < fileSize;c++){ 
        putc(((fileBuffer[c])),fpo);
    }

    fclose(fpi);
    fclose(fpo);
    free(fileBuffer);
    return 0;
}

int main(int argc, char*argv[]){
    if(argc == 3){
        if(xorFile(argv[1],argv[2]) == 0)
            printf("File encryption was successful.");
        else
            printf("An error occured.");
    }else{
        printf("usage --- xor [input file][output file]");
    }
}
6
  • See How to Ask. No offence, but we are not interested in your history. Please just post the relevant information required to answer your question. Commented Mar 1, 2016 at 22:11
  • If you're interested in security, start by paying attention to compiler messages. Then think about what your actually writing, e.g. what does FILE_BUFFER[i] != '\0' test and why are you testing it?
    – M.M
    Commented Mar 1, 2016 at 22:17
  • Please format your code properly. Commented Mar 1, 2016 at 22:18
  • 1
    Some comments on your style: 1. Use all UPPER_CASE for constants only - that is kind of an unwritten rule. 2. Don't use stderr for "normal" input/output. It is intended for error messages, hence the name. 3. The '\0' as an end marker is probably not so a good idea, after all - Your file might contain that character. You have allocated that buffer and have a byte count of characters in it - Why don't you use that? 4. Never, NEVER, use malloc() without a properly associated free() 5. What happens if input and output file name are identical? Think about it.... 6. FILE has been mentioned.
    – tofro
    Commented Mar 1, 2016 at 22:31
  • @M.M I appreciated that comment extensively... And it all made sense as to why I was only getting a single line xor'd. Commented Mar 2, 2016 at 0:05

2 Answers 2

0

Your prototype for XOR_FILE is incorrect: you should take 2 strings.

There are more issues in your code:

  • you must learn to indent your code and use spaces wisely. Use the Kernighan and Ritchie style shown in the book.
  • you cannot reliably get the file size with fseek and ftell, it is not needed in general and you can implement a bufferized version with a fixed sized buffer anyway.
  • avoid overwriting the input file in your program. If you make a mistake, or if the program fails or is interrupted, the file contents may be corrupted or lost.
  • you do not need to null terminate the array into which you read the file, just iterate over all bytes, but stop at the size read: use for (i = 0; i < newLen; i++) otherwise you would output an extra byte when encrypting and one more when deciphering...
  • do not iterate until '\0' in XOR_BUFFER(char *FILE_BUFFER), pass the size and use it. Otherwise you will fail to encrypt binary files that contain null bytes.
  • you forget to close fpo with fclose(fpo);
  • do not redefine standard functions such as getchar() and putchar().
  • do not use uppercase letters for function names and/or variable names, but it is indeed common practice to use uppercase letters for macros.

Here is a simplified version:

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

#define XOR_BYTE   0x9E
                    
int xor_file(const char *infile, const char *outfile) {
    FILE *fpi, *fpo;
    int c;

    if ((fpi = fopen(infile, "rb")) == NULL) {
        fprintf(stderr, "cannot open input file %s: %s\n", infile, strerror(errno));
        return 1;
    }
    if ((fpo = fopen(outfile, "wb")) == NULL) {
        fprintf(stderr, "cannot open output file %s: %s\n", outfile, strerror(errno));
        fclose(fpi);
        return 2;
    }

    while ((c = getc(fpi)) != EOF) {
        putc(c ^ XOR_BYTE, fpo);
    }
    fclose(fpi);
    fclose(fpo);
    return 0;
}

int main(int argc, char *argv[]) {
    int status;

    if (argc == 3) {
        status = xor_file(argv[1], argv[2]);
    } else {
        fprintf(stderr, "usage: xor_file input_file output_file\n");
        status = 3;
    }
    //getch();  // avoid the need for this by running your program in the terminal
    return status;
}
7
  • Could you possibly elaborate a little bit on your second bulletin? Commented Mar 2, 2016 at 0:11
  • @JoshGolden: getchar() and putchar() are standard functions/macros defined in <stdio.h> K&R illustrates the macro facility in the C book, but you should not redefine these in your own code.
    – chqrlie
    Commented Mar 2, 2016 at 1:13
  • @JoshGolden: in the above code, I validate the number of command line arguments, pass the filenames to xor_file(), open the files, check for success, and run a very simple loop, reading one byte at a time and writing the xor'ed value to the output file.
    – chqrlie
    Commented Mar 2, 2016 at 1:17
  • yea I understood the code you speak of I was just wondering how else I could get the size of a file w/o using fseek and ftell. Commented Mar 2, 2016 at 3:19
  • @JoshGolden: there are ways to get the size of a file via OS specific calls, but a stream is not necessarily attached to a file, and even if it is, the number of bytes you can read from a file may be different from its byte size, for instance because of linefeed translation as happens on Windows. It is much better to just read from the stream until EOF and use flexible methods to handle the data as it comes.
    – chqrlie
    Commented Mar 2, 2016 at 7:58
0

well you are passing strings into a function that expects FILE* parameters. This tells me that the compiler is complaining big time at you and you are ignoring it

Also you are not testing any return values from your fopen functions.

So fix those 2 things then repost

2
  • 1
    This is a comment, not an answer. Commented Mar 1, 2016 at 22:13
  • @pm100 Thank's alot man for pointing me in the right direction right away! Commented Mar 2, 2016 at 0:10

Not the answer you're looking for? Browse other questions tagged or ask your own question.