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

Memory leaks in CVRF module #973

Closed jan-cerny closed 9 months ago

jan-cerny commented 6 years ago

Coverity scan of OpenSCAP 1.2.16 has reported numerous issues in CVRF module.

It is possible that some of them have already been mitigated by https://github.com/OpenSCAP/openscap/pull/933. But that needs to be investigated.

Feel free to fix only some of them, you don't have to do everything in a single PR πŸ˜ƒ


1. openscap-1.2.16/src/CVRF/cvrf_priv.c:2216: alloc_fn: Storage is returned from allocation function "cvrf_doc_publisher_new".
2. openscap-1.2.16/src/CVRF/cvrf_priv.c:1080:33: alloc_fn: Storage is returned from allocation function "malloc".
3. openscap-1.2.16/src/CVRF/cvrf_priv.c:1080:33: var_assign: Assigning: "ret" = "malloc(32UL)".
6. openscap-1.2.16/src/CVRF/cvrf_priv.c:1088:2: return_alloc: Returning allocated memory "ret".
7. openscap-1.2.16/src/CVRF/cvrf_priv.c:2216: var_assign: Assigning: "publisher" = storage returned from "cvrf_doc_publisher_new()".
10. openscap-1.2.16/src/CVRF/cvrf_priv.c:2220: leaked_storage: Variable "publisher" going out of scope leaks the storage it points to.
#  2218|    if (publisher->type == CVRF_DOC_PUBLISHER_UNKNOWN && xmlTextReaderIsEmptyElement(reader) == 1) {
#  2219|        cvrf_set_parsing_error("DocumentPublisher");
#  2220|->      return NULL;
#  2221|    }
#  2222|    publisher->vendor_id = (char *)xmlTextReaderGetAttribute(reader, ATTR_VENDOR_ID);
1. openscap-1.2.16/src/CVRF/cvrf_priv.c:2146: alloc_fn: Storage is returned from allocation function "cvrf_doc_tracking_new".
2. openscap-1.2.16/src/CVRF/cvrf_priv.c:1016:32: alloc_fn: Storage is returned from allocation function "malloc".
3. openscap-1.2.16/src/CVRF/cvrf_priv.c:1016:32: var_assign: Assigning: "ret" = "malloc(72UL)".
6. openscap-1.2.16/src/CVRF/cvrf_priv.c:1029:2: return_alloc: Returning allocated memory "ret".
7. openscap-1.2.16/src/CVRF/cvrf_priv.c:2146: var_assign: Assigning: "tracking" = storage returned from "cvrf_doc_tracking_new()".
9. openscap-1.2.16/src/CVRF/cvrf_priv.c:2149: leaked_storage: Variable "tracking" going out of scope leaks the storage it points to.
#  2147|    if (xmlTextReaderIsEmptyElement(reader) == 1) {
#  2148|        cvrf_set_parsing_error("DocumentTracking");
#  2149|->      return NULL;
#  2150|    }
#  2151|   
1. openscap-1.2.16/src/CVRF/cvrf_priv.c:2088: alloc_fn: Storage is returned from allocation function "cvrf_note_new".
2. openscap-1.2.16/src/CVRF/cvrf_priv.c:910:24: alloc_fn: Storage is returned from allocation function "malloc".
3. openscap-1.2.16/src/CVRF/cvrf_priv.c:910:24: var_assign: Assigning: "ret" = "malloc(32UL)".
6. openscap-1.2.16/src/CVRF/cvrf_priv.c:919:2: return_alloc: Returning allocated memory "ret".
7. openscap-1.2.16/src/CVRF/cvrf_priv.c:2088: var_assign: Assigning: "note" = storage returned from "cvrf_note_new()".
9. openscap-1.2.16/src/CVRF/cvrf_priv.c:2091: leaked_storage: Variable "note" going out of scope leaks the storage it points to.
#  2089|    if (xmlTextReaderIsEmptyElement(reader) == 1) {
#  2090|        cvrf_set_parsing_error("Note");
#  2091|->      return NULL;
#  2092|    }
#  2093|   

1. openscap-1.2.16/src/CVRF/cvrf_priv.c:1983: alloc_fn: Storage is returned from allocation function "cvrf_product_tree_new".
2. openscap-1.2.16/src/CVRF/cvrf_priv.c:768:32: alloc_fn: Storage is returned from allocation function "malloc".
3. openscap-1.2.16/src/CVRF/cvrf_priv.c:768:32: var_assign: Assigning: "ret" = "malloc(32UL)".
6. openscap-1.2.16/src/CVRF/cvrf_priv.c:776:2: return_alloc: Returning allocated memory "ret".
7. openscap-1.2.16/src/CVRF/cvrf_priv.c:1983: var_assign: Assigning: "tree" = storage returned from "cvrf_product_tree_new()".
9. openscap-1.2.16/src/CVRF/cvrf_priv.c:1986: leaked_storage: Variable "tree" going out of scope leaks the storage it points to.
#  1984|    if (xmlTextReaderIsEmptyElement(reader) == 1) {
#  1985|        cvrf_set_parsing_error("ProductTree");
#  1986|->      return NULL;
#  1987|    }
#  1988|    xmlTextReaderNextElementWE(reader, TAG_PRODUCT_TREE);

7. openscap-1.2.16/src/CVRF/cvrf_priv.c:1659: alloc_fn: Storage is returned from allocation function "oscap_element_string_copy".
13. openscap-1.2.16/src/common/elements.c:129:3: alloc_fn: Storage is returned from allocation function "calloc".
14. openscap-1.2.16/src/common/elements.c:129:3: return_alloc_fn: Directly returning storage allocated by "calloc".
15. openscap-1.2.16/src/CVRF/cvrf_priv.c:1659: noescape: Resource "oscap_element_string_copy(reader)" is not freed or pointed-to in "oscap_stringlist_add_string".
16. openscap-1.2.16/src/common/list.c:353:77: noescape: "oscap_stringlist_add_string(struct oscap_stringlist *, char const *)" does not free or save its parameter "str".
17. openscap-1.2.16/src/CVRF/cvrf_priv.c:1659: leaked_storage: Failing to save or free storage allocated by "oscap_element_string_copy(reader)" leaks it.
#  1657|            score_set->vector = oscap_element_string_copy(reader);
#  1658|        } else if (xmlStrcmp(xmlTextReaderConstLocalName(reader), TAG_PRODUCT_ID) == 0) {
#  1659|->          oscap_stringlist_add_string(score_set->product_ids, oscap_element_string_copy(reader));
#  1660|        } else if (xmlStrcmp(xmlTextReaderConstLocalName(reader), TAG_BASE_SCORE) == 0) {
#  1661|            cvrf_score_set_add_metric(score_set, CVSS_BASE, oscap_element_string_copy(reader));

1. openscap-1.2.16/src/CVRF/cvrf_priv.c:515: alloc_fn: Storage is returned from allocation function "oscap_stringlist_new".
2. openscap-1.2.16/src/common/list.c:360:5: alloc_fn: Storage is returned from allocation function "oscap_list_new".
3. openscap-1.2.16/src/common/list.c:37:26: alloc_fn: Storage is returned from allocation function "calloc".
4. openscap-1.2.16/src/common/list.c:37:26: var_assign: Assigning: "list" = "calloc(1UL, 24UL)".
5. openscap-1.2.16/src/common/list.c:38:2: return_alloc: Returning allocated memory "list".
6. openscap-1.2.16/src/common/list.c:360:5: return_alloc_fn: Directly returning storage allocated by "oscap_list_new".
7. openscap-1.2.16/src/CVRF/cvrf_priv.c:515: var_assign: Assigning: "filtered_ids" = storage returned from "oscap_stringlist_new()".
10. openscap-1.2.16/src/CVRF/cvrf_priv.c:540: leaked_storage: Variable "filtered_ids" going out of scope leaks the storage it points to.
#   538|    }
#   539|    cvrf_product_status_iterator_free(statuses);
#   540|->  return ret;
#   541|   }
#   542|   
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.