Deallocation of control->suffix corrupts Heap Memory

[pietroborrello]

The suffix field in the static rzip_control structure is initialized to point to global memory in initialize_control

lrzip/lrzip.c

Line 1341 in 64eb4a8

control->suffix = ".lrz";

and in the lrzip main.

lrzip/main.c

Line 496 in 6a1600b

control->suffix = optarg;

However the field is then treated as a heap allocated variable while freeing the rzip_control variable.
Both in rzip_control_free

lrzip/rzip.c

Line 1269 in 465afe8

dealloc(control->suffix);

and when setting a new suffix

lrzip/liblrzip.c

Line 439 in 465afe8

dealloc(lr->control->suffix);

Impact

Corrupting the heap state may result in an exploitable vulnerability, especially if initialized with optarg that points to global RW memory.

Fix
It is sufficient to initialize control->suffix using the return value of a strdup of the strings.

[pete4abw]

Good grief! This has been around since v0.1 and rzip before, even before I became involved (v0.19). The initialise function should be used for setting constants or like-size variables, like compression level, etc. Setting control->suffix to equal optarg is probably a mistake if there will be recursion. I think the dealloc of suffix is incorrect too. It does not need to be. HOWEVER, the ability to pipe input to lrzip sort of makes recursion obsolete and unnecessary. strdup will work and I'll see about implementing it in lrzip-next. Thank you

[pietroborrello]

Great, thank you! Will checkout lrzip-next

[ckolivas]

Fixed in master.

[carnil]

Retrospective note: This seems to have been a CVE assigned, which is CVE-2022-28044.

[utkarsh2102]

Hello, is there a simple reproducer for this one?