Strncat causes exception

0

I am making a program in c to read a file and execute different pieces of code depending on what is in the file. However, when I try to read the file I get an exception:

Exception thrown at 0x7C3306DD (ucrtbased.dll) in MacroTool.exe: 0xC0000005: Access violation reading location 0x00000068.

Here is my code:

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

int main(int argc, char* argv[]) {
    if (argc < 2) {
        printf("ERR: too few arguments.\n");
        return 1;
    }
    if (argc > 2) {
        printf("ERR: too many arguments.\n");
        return 1;
    }

    FILE* fp = fopen(argv[1], "r");

    if (fp == NULL) {
        printf("ERR: cannot read file.\n");
        return 1;
    }

    char buf[100];

    char c;

    while ((c = fgetc(fp)) != EOF)
        strncat(buf, c, 1);

    fclose(fp);

    return 0;
}

Using multiple search engines to find an answer that is related to this issue led to nothing.

I am using windows 10 and visual studio 2019

c
asked on Stack Overflow Jan 23, 2021 by Finxx

2 Answers

3

You are attempting to use a string-funciton on a buffer that is not nul-terminated resulting in Undefined Behavior. Specifically:

    char buf[100];

    char c;

    while ((c = fgetc(fp)) != EOF)
        strncat(buf, c, 1);

On your first call to strncat, buf in uninitialized resulting in Undefined Behavior due to strncat() replacing the existing nul-terminating character (which doesn't exist) with the new text to be appended. You can initialize buf all zero with:

    char buf[100] = "";

That will fix the immediate Undefined Behavior but will not prevent you later reading beyond the bounds of buf when you read past the 99th character.

Instead, declare a counter and initialize the counter zero, and use that to limit the number of characters read into your buf, as in the comment:

    size_t n = 0;
    
    while (n + 1 < 100 && (c = fgetc(fp)) != EOF) { 
        buf[n++] = c;
    }
    buf[n] = 0;      /* don't forget to nul-terminate buf */

You can put it altogether and avoid using Magic-Numbers (100) as follows:

#include <stdio.h>

#define MAXC 100        /* if you need a constant, #define one (or more) */

int main (int argc, char* argv[]) {
    
    if (argc < 2) { /* validate one argument given for filename */
        fputs ("error: too few arguments.\n", stderr);
        return 1;
    }
    char buf[MAXC] = "", c;
    size_t n = 0;
    FILE* fp = fopen (argv[1], "r");

    if (fp == NULL) {                   /* validate file open for reading */
        perror ("fopen-argv[1]");       /* on failure, perror() tells why */
        return 1;
    }

    /* while buf not full (saving 1-char for \0), read char */
    while (n + 1 < MAXC && (c = fgetc(fp)) != EOF)
        buf[n++] = c;                   /* assign char to next element in buf */

    buf[n] = 0;                         /* nul-terminate buf */
    
    puts (buf);                         /* output result */
    
    fclose(fp);
}

(note: the comparison with n + 1 ensures one-element in buf remains for the nul-terminating character)

Example Use/Output

$ ./bin/read100chars read100chars.c
#include <stdio.h>

#define MAXC 100        /* if you need a constant, #define one (or more) */

in

(note: the first 'n' in int main (... is the 99th character in the file)

Look thing over and let me know if you need further help. (note with VS you will likely need to pass /w4996 to disable the "CRT Secure...." warning.)

answered on Stack Overflow Jan 24, 2021 by David C. Rankin • edited Jan 24, 2021 by David C. Rankin
2
char *strncat(char *dest, const char *src, size_t n);

https://linux.die.net/man/3/strncat

The strcat() function appends the src string to the dest string, overwriting the terminating null byte ('\0') at the end of dest, and then adds a terminating null byte. The strings may not overlap, and the dest string must have enough space for the result. If dest is not large enough, program behavior is unpredictable; buffer overruns are a favorite avenue for attacking secure programs.

Note that strncat will take 2 pointers, first the destination and the second is the source, you're providing a character so it should be strncat(buf, &c, 1);

the strncat will start appending characters at the end of the destination, ie. at the null byte, and since your buffer isn't initialized, it contains noise and may not contain nullbyte '\0' inside. The strncat try searching the \0 starting from buff and it may find somewhere after the array ends and start to append characters there. and that's why you get error Access violation writing location 0x68FB49FC to fix this set the first byte of your buffer to \0

char buf[100];
buf[0] = '\0';

Also you don't check if your buffer has space for the new character, which also leads to access violation that if your source file contains more than 99 characters. you also have to handle that as well

  1. limit your loop by 99 characters (and print an error if it's exceeded)
  2. make your buffer enough like char buf[2048]; or something but still, it'll crash if it's not enough
  3. create a buffer with the size of your source
size_t getFileSize(FILE* _file) {
    fseek(_file, 0, SEEK_END);
    size_t _size = ftell(_file);
    fseek(_file, 0, SEEK_SET);
    return _size;
}
char* buf = malloc(getFileSize(fp)); //< you have to manage the allocated memory 

And one more thing

you're trying to read a file into a buffer by strncat this might be in efficient since every time it needs to get the strlen, check the size n and add both character and \0. Instead, I recommend

char* readFile(FILE* _file) {
    size_t file_size = getFileSize(_file);

    char* buf = malloc(file_size + 1);
    size_t read = fread(buf, sizeof(char), file_size, _file);
    buf[read] = '\0';

    return buf;
}
answered on Stack Overflow Jan 24, 2021 by thakee nathees • edited Jan 24, 2021 by thakee nathees

User contributions licensed under CC BY-SA 3.0