Invalid memory approach (heap-use-after-free)

[gaoxiang-ut]

When I was using this library, I used a security detection tool to detect a memory leak problem

[Joungkyun]

Since the original code 160 line is approached to the removed variable, the 156 line must be removed.

[Joungkyun]

I think missing following code lilne:

(*obj)->bom = det->getIsBOM ();

[Joungkyun]

The det variable used by the detect_handledata_r function was generated by the detect_init function. Therefore, depending on the return value of detect_Handledata_r, it must be deleted by the detect_destroy function outside the detect_handledata_r.

Therefore, the detect_dataHandle_r function modification is not required as my thought.

For example, you must be processed as follows:

d = detect_init ();
detect_reset (&d);

obj = detect_obj_init ();
swith (detect_handledata_r (&d, strings, string_length, &obj)) {
    case CHARDET_OUT_OF_MEMORY :
    case CHARDET_NULL_OBJECT :
        detect_obj_free (&obj);
        detect_destroy (&d);
        exit (1);
}
detect_obj_free (&obj);
detect_destroy (&d);

The detect_handledata_r function is provided for use in the loop. Therefore, the example of the following example is better:

d = detect_init ();
if ( !d ) exit (1);

while ( condition ) {
    detect_reset (&d);
    obj = detect_obj_init ();

    swith (detect_handledata_r (&d, strings, string_length, &obj)) {
        case CHARDET_OUT_OF_MEMORY :
        case CHARDET_NULL_OBJECT :
            detect_obj_free (&obj);
            continue;
    }

    printf (
        "encoding: %s, confidence: %f, exist BOM: %d\n",
        obj->encoding, obj->confidence, obj->bom
    );
    detect_obj_free (&obj);
}
detect_destroy (&d);

[Joungkyun]

When you create a patch, use Indent as TAB.

[gaoxiang-ut]

I have modified it in accordance with your comments

[Joungkyun]

Why do you think that this issue is security vulnerablities?
151 line has been freed memory of det, and it is a problem that you are accessing freeed det from the 160 line.

151  delete det;
...
160 (*obj)->bom = det->getIsBOM ();

If you can insert a code that can be attacked by this freed memory address, it will be a security issue, but it seems that there is no path to insert the code into that memory.
So this problem is that SEGFAULT can be occured by accessing invalid memory rather than secure issues.

[Joungkyun]

The patch seems that there is no problem.
However, I asked to ask for the indentation of the code before.
libchardet uses tabs instead of white spaces for indentation.

[gaoxiang-ut]

I verified it according to your sample1.c example, added safety detection parameters, and prompted heap-use-after-free during actual operation

[gaoxiang-ut]

compilation parameters:
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -g -fsanitize=address -O2")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g -fsanitize=address -O2")

operation result:
==20703==ERROR: AddressSanitizer: heap-use-after-free on address 0x608000001ad0 at pc 0x0000004057b1 bp 0x7ffc3a433c80 sp 0x7ffc3a433c78
READ of size 4 at 0x608000001ad0 thread T0
#0 0x4057b0 in detect_r /home/gaoxiang/Work_GX/dvp/demo/libchardetDemo/src/chardet.cpp:177
#1 0x402cab in main /home/gaoxiang/Work_GX/dvp/demo/libchardetDemo/main.cpp:47
#2 0x7fe54c5e309a in __libc_start_main ../csu/libc-start.c:308
#3 0x403699 in _start (/home/gaoxiang/Work_GX/dvp/demo/build-libchardetDemo-unknown-Debug/libchardetDemo+0x403699)

[Joungkyun]

I will change this PR title. Do not change again. I think the issue title should summarize the contents of the issue of the issue. Security vulnerabilities are too broad.
Regardless of this, I think it is very grateful to your patch.