OpenSCAP / openscap

NIST Certified SCAP 1.2 toolkit
https://www.open-scap.org/tools/openscap-base
GNU Lesser General Public License v2.1
1.38k stars 380 forks source link

Refactor cvrf_index_parse_xml to prevent storage leaks #1190

Closed jan-cerny closed 4 years ago

jan-cerny commented 6 years ago

Our static analysis reports a resource leak.

1. openscap-1.2.17/utils/oscap-cvrf.c:121: alloc_fn: Storage is returned from allocation function "oscap_source_new_from_file".
2. openscap-1.2.17/src/source/oscap_source.c:81:30: alloc_fn: Storage is returned from allocation function "calloc".
3. openscap-1.2.17/src/source/oscap_source.c:81:30: var_assign: Assigning: "source" = "calloc(1UL, 56UL)".
4. openscap-1.2.17/src/source/oscap_source.c:84:2: return_alloc: Returning allocated memory "source".
5. openscap-1.2.17/utils/oscap-cvrf.c:121: var_assign: Assigning: "import_source" = storage returned from "oscap_source_new_from_file(action->cvrf_action->f_cvrf)".
10. openscap-1.2.17/utils/oscap-cvrf.c:138: noescape: Resource "import_source" is not freed or pointed-to in "cvrf_model_import".
11. openscap-1.2.17/src/CVRF/cvrf.c:58:59: noescape: "cvrf_model_import(struct oscap_source *)" does not free or save its parameter "source".
16. openscap-1.2.17/utils/oscap-cvrf.c:158: leaked_storage: Variable "import_source" going out of scope leaks the storage it points to.
#   156|     */
#   157|    free(action->cvrf_action);
#   158|->  return result;
#   159|   }
#   160|   

However, it is not possible to simply free the pointer, as is already stated in a comment:

https://github.com/OpenSCAP/openscap/blob/3fae9162ab0fe044547b287af863da15c4a5fafb/utils/oscap-cvrf.c#L154-L155

evgenyz commented 4 years ago

@jan-cerny This one might be interesting from OOM epic point of view. Also, please close it if is already fixed (or not applicable) in 1.3.x.

evgenyz commented 4 years ago

Scratch that. Fixed in #1359.