From 879e67e5f6fa01f5ae745c9287e95f74541d32ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Mon, 19 Jul 2021 14:53:04 +0200 Subject: [PATCH 1/3] Fix several covscan warnings - don't reuse `tmp` variable across the entire function - don't reuse `it` variable across the entire function - check whether malloc was successful - do not copy strings unnecessarily - remove couple of macros which were confusing covscans and are too verbose - add missing return value checks --- libcomps/src/comps_doc.c | 7 +++---- libcomps/src/comps_doccategory.c | 9 +++++++-- libcomps/src/comps_docenv.c | 10 ++++++++-- libcomps/src/comps_docgroup.c | 9 +++++++-- libcomps/src/comps_parse.c | 12 ++++-------- libcomps/src/comps_set.c | 18 ++++++++++-------- libcomps/src/python/src/pycomps.c | 5 ++--- libcomps/src/python/src/pycomps_sequence.c | 13 +++++++++++++ 8 files changed, 54 insertions(+), 29 deletions(-) diff --git a/libcomps/src/comps_doc.c b/libcomps/src/comps_doc.c index 9e6005b..c5e65cd 100644 --- a/libcomps/src/comps_doc.c +++ b/libcomps/src/comps_doc.c @@ -644,7 +644,6 @@ static signed char comps_doc_xml(COMPS_Doc *doc, xmlTextWriterPtr writer, COMPS_ObjMDict *mdict; COMPS_HSList *hslist; COMPS_HSListItem *hsit; - char *tmp; int retc; signed char ret = 0, tmpret; @@ -709,7 +708,7 @@ static signed char comps_doc_xml(COMPS_Doc *doc, xmlTextWriterPtr writer, xmlTextWriterWriteAttribute(writer, BAD_CAST "name", (xmlChar*) ((COMPS_ObjRTreePair*)hsit->data)->key); - tmp = comps_object_tostr(((COMPS_ObjRTreePair*)hsit->data)->data); + char *tmp = comps_object_tostr(((COMPS_ObjRTreePair*)hsit->data)->data); xmlTextWriterWriteAttribute(writer, BAD_CAST "install", BAD_CAST tmp); free(tmp); @@ -749,7 +748,7 @@ static signed char comps_doc_xml(COMPS_Doc *doc, xmlTextWriterPtr writer, xmlTextWriterWriteAttribute(writer, BAD_CAST "name", (xmlChar*) ((COMPS_ObjRTreePair*)hsit->data)->key); - tmp = comps_object_tostr(it->comps_obj); + char *tmp = comps_object_tostr(it->comps_obj); xmlTextWriterWriteAttribute(writer, BAD_CAST "arch", BAD_CAST tmp); free(tmp); @@ -789,7 +788,7 @@ static signed char comps_doc_xml(COMPS_Doc *doc, xmlTextWriterPtr writer, xmlTextWriterWriteAttribute(writer, BAD_CAST "requires", (xmlChar*) ((COMPS_ObjRTreePair*)hsit->data)->key); - tmp = comps_object_tostr(it->comps_obj); + char *tmp = comps_object_tostr(it->comps_obj); xmlTextWriterWriteAttribute(writer, BAD_CAST "package", BAD_CAST tmp); free(tmp); diff --git a/libcomps/src/comps_doccategory.c b/libcomps/src/comps_doccategory.c index 9c36633..59f9772 100644 --- a/libcomps/src/comps_doccategory.c +++ b/libcomps/src/comps_doccategory.c @@ -317,8 +317,13 @@ char* comps_doccategory_tostr_u(COMPS_Object* cat) { total_len += strlen(desc_by_lang_str); group_ids_str = comps_object_tostr((COMPS_Object*)_cat_->group_ids); total_len += strlen(group_ids_str); - - ret = malloc(sizeof(char) * (total_len+2+(6*2)+strlen(head))); + + if ((ret = malloc(sizeof(char) * (total_len+2+(6*2)+strlen(head)))) == NULL) { + free(name_by_lang_str); + free(desc_by_lang_str); + free(group_ids_str); + return NULL; + } ret[0] = 0; strcat(ret, head); for (int i=0; i<4; i++) { diff --git a/libcomps/src/comps_docenv.c b/libcomps/src/comps_docenv.c index 11b9b72..c93d7be 100644 --- a/libcomps/src/comps_docenv.c +++ b/libcomps/src/comps_docenv.c @@ -415,8 +415,14 @@ char* comps_docenv_tostr_u(COMPS_Object* env) { total_len += strlen(group_list_str); option_list_str = comps_object_tostr((COMPS_Object*)_env_->option_list); total_len += strlen(option_list_str); - - ret = malloc(sizeof(char) * (total_len+2+(8*2)+strlen(head))); + + if ((ret = malloc(sizeof(char) * (total_len+2+(8*2)+strlen(head)))) == NULL) { + free(name_by_lang_str); + free(desc_by_lang_str); + free(group_list_str); + free(option_list_str); + return NULL; + } ret[0] = 0; strcat(ret, head); for (int i=0; i<4; i++) { diff --git a/libcomps/src/comps_docgroup.c b/libcomps/src/comps_docgroup.c index 6c0eb14..6f89ed6 100644 --- a/libcomps/src/comps_docgroup.c +++ b/libcomps/src/comps_docgroup.c @@ -399,8 +399,13 @@ char* comps_docgroup_tostr_u(COMPS_Object* group) { total_len += strlen(desc_by_lang_str); group_packages_str = comps_object_tostr((COMPS_Object*)_group_->packages); total_len += strlen(group_packages_str); - - ret = malloc(sizeof(char) * (total_len+2+(8*2)+strlen(head))); + + if ((ret = malloc(sizeof(char) * (total_len+2+(8*2)+strlen(head)))) == NULL) { + free(name_by_lang_str); + free(desc_by_lang_str); + free(group_packages_str); + return NULL; + } ret[0] = 0; strcat(ret, head); for (int i=0; i<6; i++) { diff --git a/libcomps/src/comps_parse.c b/libcomps/src/comps_parse.c index 55dbd2f..18133a2 100644 --- a/libcomps/src/comps_parse.c +++ b/libcomps/src/comps_parse.c @@ -436,19 +436,16 @@ void comps_parse_char_data_handler(void *userData, } void comps_parse_check_attributes(COMPS_Parsed *parsed, COMPS_Elem* elem) { - #define parser_line XML_GetCurrentLineNumber(parsed->parser) - #define parser_col XML_GetCurrentColumnNumber(parsed->parser) const COMPS_ElemInfo *info; info = COMPS_ElemInfos[elem->type]; int attr_count; COMPS_HSList *keys; char *val; - COMPS_HSListItem *it; for (attr_count = 0; info->attributes[attr_count] != NULL; attr_count++); keys = comps_dict_keys(elem->attrs); for (int x =0; xfirst; it != NULL; it = it->next) { + for (COMPS_HSListItem *it = keys->first; it != NULL; it = it->next) { if (strcmp((char*)it->data, info->attributes[x]->name) == 0) { if (info->attributes[x]->val_check) { val = comps_dict_get(elem->attrs, it->data); @@ -464,12 +461,11 @@ void comps_parse_check_attributes(COMPS_Parsed *parsed, COMPS_Elem* elem) { } } } - for (it = keys->first; it != NULL; it = it->next) { + for (COMPS_HSListItem *it = keys->first; it != NULL; it = it->next) { comps_log_warning_x(parsed->log, COMPS_ERR_ATTR_UNKNOWN, 4, comps_str(it->data), comps_str(info->name), - comps_num(parser_line), comps_num(parser_col)); + comps_num(XML_GetCurrentLineNumber(parsed->parser)), + comps_num(XML_GetCurrentColumnNumber(parsed->parser))); } comps_hslist_destroy(&keys); - #undef parser_line - #undef parser_col } diff --git a/libcomps/src/comps_set.c b/libcomps/src/comps_set.c index e3eecfa..9ea048a 100644 --- a/libcomps/src/comps_set.c +++ b/libcomps/src/comps_set.c @@ -113,14 +113,16 @@ char comps_set_add(COMPS_Set * set, void *item) { } void* comps_set_remove(COMPS_Set *set, void *item) { - void * ret; - COMPS_HSListItem * it; - for (it = set->data->first; it != NULL; it = it->next) { - if (set->eqf(it->data, item)) { - comps_hslist_remove(set->data, it); - ret = it->data; - free(it); - return ret; + if (set && set->data) { + void * ret; + COMPS_HSListItem * it; + for (it = set->data->first; it != NULL; it = it->next) { + if (set->eqf(it->data, item)) { + comps_hslist_remove(set->data, it); + ret = it->data; + free(it); + return ret; + } } } return NULL; diff --git a/libcomps/src/python/src/pycomps.c b/libcomps/src/python/src/pycomps.c index aa73a8e..1c1cb3b 100644 --- a/libcomps/src/python/src/pycomps.c +++ b/libcomps/src/python/src/pycomps.c @@ -499,7 +499,6 @@ PyObject* PyCOMPS_filter_arches(PyObject *self, PyObject *other) { PyCOMPS *doc; COMPS_Doc *comps_doc; PyObject *item; - char *str; char created = 0; if ((Py_TYPE(other) != &PyCOMPS_StrSeqType) && (Py_TYPE(other) != &PyList_Type)) { @@ -512,9 +511,9 @@ PyObject* PyCOMPS_filter_arches(PyObject *self, PyObject *other) { arches = COMPS_OBJECT_CREATE(COMPS_ObjList, NULL); for (Py_ssize_t x=0; x < PyList_Size(other); x++) { item = PyList_GetItem(other, x); + char *str; __pycomps_arg_to_char(item, &str); - comps_objlist_append_x(arches, (COMPS_Object*)comps_str(str)); - free(str); + comps_objlist_append_x(arches, (COMPS_Object*)comps_str_x(str)); } } else { arches = ((PyCOMPS_Sequence*)other)->list; diff --git a/libcomps/src/python/src/pycomps_sequence.c b/libcomps/src/python/src/pycomps_sequence.c index acd2e71..4f5f886 100644 --- a/libcomps/src/python/src/pycomps_sequence.c +++ b/libcomps/src/python/src/pycomps_sequence.c @@ -307,12 +307,14 @@ int list_set_slice(PyObject *self, PyObject *key, PyObject *val) { n = _seq_->list->len; uret = PySlice_GetIndicesEx((PyObject*)key, n, &istart, &istop, &istep, &ilen); + if (uret) return -1; if (ilen == 0) { uret = PySlice_GetIndicesEx((PyObject*)key, n+istart, &istart, &istop, &istep, &ilen); } if (uret) return -1; if (val) { + // set val for list items indexed by given slice if (Py_TYPE(self) != Py_TYPE(val)) { PyErr_SetString(PyExc_TypeError, "different object class"); return -1; @@ -340,6 +342,11 @@ int list_set_slice(PyObject *self, PyObject *key, PyObject *val) { for (i=0 ; inext, i++); if (istep != 1) { while (clen != ilen) { + if (!it) { + PyErr_SetString(PyExc_ValueError, + "failed to index list using the given slice"); + return -1; + } COMPS_OBJECT_DESTROY(it->comps_obj); it->comps_obj = comps_object_incref(it2->comps_obj); clen += 1; @@ -372,10 +379,16 @@ int list_set_slice(PyObject *self, PyObject *key, PyObject *val) { } return 0; } else { + // if val is NULL we want to delete list items indexed by given slice clen = 0; it = ((PyCOMPS_Sequence*)self)->list->first; for (i=0 ; inext, i++); while (clen != ilen) { + if (!it) { + PyErr_SetString(PyExc_ValueError, + "failed to index list using the given slice"); + return -1; + } if (it->comps_obj) { COMPS_OBJECT_DESTROY(it->comps_obj); it->comps_obj = NULL; From 50f3b3b80c8f21987ddfc726bcc75980e0db1b95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Tue, 20 Jul 2021 11:44:35 +0200 Subject: [PATCH 2/3] Add missing undef for _seq_ macro and use it everywhere --- libcomps/src/python/src/pycomps_sequence.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/libcomps/src/python/src/pycomps_sequence.c b/libcomps/src/python/src/pycomps_sequence.c index 4f5f886..9b42341 100644 --- a/libcomps/src/python/src/pycomps_sequence.c +++ b/libcomps/src/python/src/pycomps_sequence.c @@ -352,7 +352,7 @@ int list_set_slice(PyObject *self, PyObject *key, PyObject *val) { clen += 1; it2 = it2->next; for (i=0 ; inext, i++); - if (!it) it = ((PyCOMPS_Sequence*)self)->list->first; + if (!it) it = _seq_->list->first; for (; inext, i++); } } else { @@ -366,14 +366,12 @@ int list_set_slice(PyObject *self, PyObject *key, PyObject *val) { } if (it == NULL) { for (;it2 != NULL; it2 = it2->next) { - comps_objlist_append(((PyCOMPS_Sequence*)self)->list, - it2->comps_obj); + comps_objlist_append(_seq_->list, it2->comps_obj); } } if (it != NULL) { for (c = i; c < istop; c++) { - comps_objlist_remove_at(((PyCOMPS_Sequence*)self)->list, - i); + comps_objlist_remove_at(_seq_->list, i); } } } @@ -381,7 +379,7 @@ int list_set_slice(PyObject *self, PyObject *key, PyObject *val) { } else { // if val is NULL we want to delete list items indexed by given slice clen = 0; - it = ((PyCOMPS_Sequence*)self)->list->first; + it = _seq_->list->first; for (i=0 ; inext, i++); while (clen != ilen) { if (!it) { @@ -395,23 +393,24 @@ int list_set_slice(PyObject *self, PyObject *key, PyObject *val) { } clen+=1; for (i=0 ; inext, i++); - if (!it) it = ((PyCOMPS_Sequence*)self)->list->first; + if (!it) it = _seq_->list->first; for (; inext, i++); } it2 = NULL; - for (i=0, it = ((PyCOMPS_Sequence*)self)->list->first; + for (i=0, it = _seq_->list->first; it != NULL; it2 = it, it = it->next, i++) { if (it2 && !it2->comps_obj) { - comps_objlist_remove_at(((PyCOMPS_Sequence*)self)->list, i); + comps_objlist_remove_at(_seq_->list, i); } } if (it2 && !it2->comps_obj) { - comps_objlist_remove_at(((PyCOMPS_Sequence*)self)->list, i); + comps_objlist_remove_at(_seq_->list, i); } return 0; } } return 0; + #undef _seq_ } int __PyCOMPSSeq_set(PyObject *self, PyObject *key, PyObject *val, From e7521d21bba6407957325a63cb9d65c07a2e0a94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Tue, 10 Aug 2021 08:12:28 +0200 Subject: [PATCH 3/3] Check return value of `__pycomps_arg_to_char` and add a test It fixes a crash when the conversion to char is not successful. --- libcomps/src/python/src/pycomps.c | 5 ++++- libcomps/src/python/tests/__test.py | 5 +++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/libcomps/src/python/src/pycomps.c b/libcomps/src/python/src/pycomps.c index 1c1cb3b..ee6f691 100644 --- a/libcomps/src/python/src/pycomps.c +++ b/libcomps/src/python/src/pycomps.c @@ -512,7 +512,10 @@ PyObject* PyCOMPS_filter_arches(PyObject *self, PyObject *other) { for (Py_ssize_t x=0; x < PyList_Size(other); x++) { item = PyList_GetItem(other, x); char *str; - __pycomps_arg_to_char(item, &str); + if (__pycomps_arg_to_char(item, &str)) { + COMPS_OBJECT_DESTROY(arches); + return NULL; + } comps_objlist_append_x(arches, (COMPS_Object*)comps_str_x(str)); } } else { diff --git a/libcomps/src/python/tests/__test.py b/libcomps/src/python/tests/__test.py index 35a41f7..4152c7d 100644 --- a/libcomps/src/python/tests/__test.py +++ b/libcomps/src/python/tests/__test.py @@ -1088,6 +1088,11 @@ def test_arches(self): comps5.fromxml_str(s) self.assertTrue(comps == comps5) + def test_arches_invalid_input(self): + INVALID_UTF8_CHAR = '\udcfd' + c = libcomps.Comps() + self.assertRaises(TypeError, c.arch_filter, [INVALID_UTF8_CHAR]) + #@unittest.skip("") def test_validate(self): c = libcomps.Comps()