3

valgrind --tool=memcheck --leak-check=full --show-reachable=yes -v ./out

==37408== ERROR SUMMARY: 23 errors from 23 contexts (suppressed: 0 from 0)
==37408== 
==37408== 1 errors in context 1 of 23:
==37408== Invalid read of size 1
==37408==    at 0x707E: memmove$VARIANT$sse42 (in /usr/local/Cellar/valgrind/3.8.1/lib/valgrind/vgpreload_memcheck-amd64-darwin.so)
==37408==    by 0x10F08F: strdup (in /usr/lib/system/libsystem_c.dylib)
==37408==    by 0x10000576A: new_node (parser.c:349)
==37408==    by 0x100005E5D: string_literal (parser.c:166)
==37408==    by 0x100001A5D: yyparse (vtl4.y:142)
==37408==    by 0x1000029F1: main (vtl4.y:218)
==37408==  Address 0x100055826 is 0 bytes after a block of size 6 alloc'd
==37408==    at 0x58D3: calloc (in /usr/local/Cellar/valgrind/3.8.1/lib/valgrind/vgpreload_memcheck-amd64-darwin.so)
==37408==    by 0x100006734: strclip (util.c:35)
==37408==    by 0x100005E4B: string_literal (parser.c:164)
==37408==    by 0x100001A5D: yyparse (vtl4.y:142)
==37408==    by 0x1000029F1: main (vtl4.y:218)
==37408== 
==37408== 
==37408== 1 errors in context 2 of 23:
==37408== Invalid read of size 1
==37408==    at 0x67A4: strlen (in /usr/local/Cellar/valgrind/3.8.1/lib/valgrind/vgpreload_memcheck-amd64-darwin.so)
==37408==    by 0x10F066: strdup (in /usr/lib/system/libsystem_c.dylib)
==37408==    by 0x10000576A: new_node (parser.c:349)
==37408==    by 0x100005E5D: string_literal (parser.c:166)
==37408==    by 0x100001A5D: yyparse (vtl4.y:142)
==37408==    by 0x1000029F1: main (vtl4.y:218)
==37408==  Address 0x100055826 is 0 bytes after a block of size 6 alloc'd
==37408==    at 0x58D3: calloc (in /usr/local/Cellar/valgrind/3.8.1/lib/valgrind/vgpreload_memcheck-amd64-darwin.so)
==37408==    by 0x100006734: strclip (util.c:35)
==37408==    by 0x100005E4B: string_literal (parser.c:164)
==37408==    by 0x100001A5D: yyparse (vtl4.y:142)
==37408==    by 0x1000029F1: main (vtl4.y:218)
==37408== 
==37408== ERROR SUMMARY: 23 errors from 23 contexts (suppressed: 0 from 0)

new_node (parser.c:349):

struct simpleNode *new_node(JJT jjt,char *node_image){
    struct simpleNode *a = malloc(sizeof(struct simpleNode));
    if (a==NULL) {
        yyerror("FUNC[%s]error:Init a new simple error,out of space!",__func__);
        exit(0);
    }
    a->info.astn = jjt;
    a->info.node_name = jjtNodeName[jjt];

    **349**>>a->info.image = strdup(node_image);
    a->info.already_rendered = cJSON_False;
    a->parent = NULL;
    a->firstChild = NULL;
    a->nextSibling = NULL;
    return a;
}

string_literal (parser.c:166):

struct simpleNode* string_literal( char *str){
    //printf("%s node!\n",__func__);
    char *strc = strclip(str, strlen(str) - 2, 1);
    //printf("**********strclip:%s\n",strc);
   **166**>> struct simpleNode* a = new_node(JJTSTRINGLITERAL,strc);
    //free(strc);
    free(str);
    str = NULL;
    return a;
}

strclip (util.c:35)

char * strclip(const char *src, unsigned long n, unsigned long m) {

    unsigned long len = strlen(src);
    if (n > len)
        n = len - m;
    if (m > len)
        return NULL ;
    const char * right_start = src + m;

   **35**<< char *q = (char *) calloc(n, sizeof(char));

    strncpy(q, right_start, n);

    return q;
}

I study for a long time didn't find the root cause of the problem, please help me!

Eugene Loy
  • 12,224
  • 8
  • 53
  • 79
sinory
  • 784
  • 3
  • 8
  • 19

1 Answers1

6

strncpy() in strclip() is the cause of your problem;

No null-character is implicitly appended at the end of destination if source is longer than num. Thus, in this case, destination shall not be considered a null terminated C string (reading it as such would overflow).

Since you're using calloc() anyway to get memory filled with zeroes, all you need to do is allocate an extra char that won't be overwritten;

char *q = (char *) calloc(n + 1, sizeof(char));
strncpy(q, right_start, n);

This will leave the last char of the buffer as zero, so you have a guaranteed termination of the string.

Joachim Isaksson
  • 176,943
  • 25
  • 281
  • 294
  • 3
    Don't use `strncpy` at all, [strncpy is not a secure version of strcpy](http://stackoverflow.com/questions/2114896/why-is-strlcpy-and-strlcat-considered-to-be-insecure). The proper, fastest-possible way to do this is malloc, memcpy and then a manual null termination. – Lundin May 29 '13 at 11:23