[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Bug-xorriso] Fixes and improvements for libisoburn

From: Eliska Svobodova
Subject: [Bug-xorriso] Fixes and improvements for libisoburn
Date: Mon, 12 Aug 2019 14:00:57 +0200

we would like to suggest a few fixes for minor bugs and compiler warnings in libisoburn. Those were found by static analysis tools like Coverity, CLang and gcc warnings.

1) Bitwise operations - shift 1 << 31 is used on multiple places through xorriso, which is an undefined operation; fix: make 1 unsigned:  1u << 31

2) Absolute value - you are using abs() to calculate the absolute value of r1count and r2count, which are of type long int, so it could be truncated; fix: use labs()

3) Dereference of NULL pointers - in xorriso/opts_d_h.c, first_job pointer can be in case of failure of Findjob_new a NULL pointer, which is dereferenced in ex:
                                                - in xorriso/opts_i_o.c, also descr can be NULL, if end_idx <= 0 and execution goes straight to ex:
                                                - in xorriso/read_run.c, when catcontent is NULL, NULL is put in buf_pt and is passed to write() function

4) Missing initialization - in xorriso/iso_manip.c, own_link_stack isn't initialized, this matters in case that there was a problem while allocating the memory and execution continues to ex: where own_link_stack is freed; fix: initialize it to NULL (that the free function can handle)

And we have some improvements to make the code safer and cleaner:

1) Resource leaks - in xorriso/drive_mgt.c, dynamically allocated disc isn't freed in case that goto ex; is used; fix: move freeing after ex:
                           - in xorriso/emulators.c and xorriso/opts_i_o.c,arguments should be freed by Sfile_destroy_argv function

2) Overflow in strcpy - in multiple files, there are strcpys which copy dynamically allocated strings into static strings, it would be better to use strncpy to avoid overflow

3) Memcpy - in libisoburn/isofs_wrap.c, strncpy is used to copy fixed-size memory without ending null (Coverity reported it as a mistake), memcpy would be probably better in order to express the intent more clearly and to avoid false-positive warning in the future.

Attachment: libisoburn_compilator_bitwise_op_labs.patch
Description: Text Data

Attachment: libisoburn_deref_NULL_pointers.patch
Description: Text Data

Attachment: libisoburn_fixed_resource_leaks.patch
Description: Text Data

Attachment: libisoburn_strcpy_to_strncpy.patch
Description: Text Data

Attachment: libisoburn_strncpy_to_memcpy.patch
Description: Text Data

Attachment: libisoburn_missing_initialization.patch
Description: Text Data

reply via email to

[Prev in Thread] Current Thread [Next in Thread]