Replacing removed secure empty trash feature

Question

Is this code reliable? Is there any extra precaution I should or should not be taking here? How is my code to securely delete a file by repeatedly overwriting the contents?

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <time.h>
int main(int Argc, const char *Argv[]) {
    srand(time(NULL));
    unsigned int *RandBuf = NULL;
    for (int I = 1; I < Argc; I++) {
        long FileSize;
        const char *const Path = Argv[I];
        FILE *const File = fopen(Path, "r+");
        if (!File || fseek(File, 0, SEEK_END) || (FileSize = ftell(File)) < 0) {
            perror(Path);
            continue;
        }
        const unsigned long IntCount = (FileSize+3)>>2;
        RandBuf = realloc(RandBuf, IntCount<<2);
        const unsigned char WriteCount = ((unsigned int)rand()&0xF)+16;
        for (unsigned char Write = 0; Write < WriteCount; Write++) {
            for (unsigned long Int = 0; Int < IntCount; Int++) {
                RandBuf[Int] ^= (unsigned int)rand();
            }
            if (fseek(File, 0, SEEK_SET) || fwrite(RandBuf, 1, FileSize, File) < FileSize) {
                perror(Path);
                continue;
            }
        }
        if (fclose(File) || remove(Path)) {
            perror(Path);
            continue;
        }
    }
    free(RandBuf);
    return 0;
}

Answer 1

Your description appears to suggest you're reimplementing wipe; I recommend you read its sources to see what an effective implementation looks like. I'll note that the concept could well be flawed if you're writing to a log-structured filesystem or to wear-levelling media - both of those need specific actions to erase data, because overwriting will simply create new data blocks elsewhere.

The "PascalCase" naming convention makes this code surprisingly hard to read (and not just because capital I looks a lot like 1, and Int sounds a lot like int). I would prefer to see more conventional identifiers, so that variables look like variables.

This include looks pointless:

 #include <unistd.h>

As we're using only Standard Library, prefer to omit this, for a portable program.

    if (!File || fseek(File, 0, SEEK_END) || (FileSize = ftell(File)) < 0) {
        perror(Path);
        continue;
    }

I approve of error checking, but there's a problem here. If we open a file but fail to seek (because it's a non-regular file, perhaps), then we fail to close it.

    RandBuf = realloc(RandBuf, IntCount<<2);

What happens when realloc() returns a null pointer? We have a leak and we have undefined behaviour when we dereference RandBuf. It seems risky to need that much memory, given that files can often be larger than address space. We'd probably prefer to write in sensible size chunks too, given the copying that's involved.

    const unsigned long IntCount = (FileSize+3)>>2;
    RandBuf = realloc(RandBuf, IntCount<<2);

Where do those magic numbers 2 and 3 originate? I'm guessing that perhaps that sizeof (int) is 4 on your platform, and the constants derive from that. But it would be better to calculate from the actual size of int and to name the constants more appropriately.

    for (unsigned char Write = 0; Write < WriteCount; Write++) {

This loop serves no purpose. We write a variety of values several times, but it's unlikely any of these values go all the way to the disk, given we never fflush() the stream.