From f147986fa96121210e892a04b74045d976689f82 Mon Sep 17 00:00:00 2001 From: wmayer Date: Sun, 16 Jan 2022 14:30:51 +0100 Subject: [PATCH] App: Bugfix for NULL-Pointer dereference of Property->getName() --- src/App/DocumentObserver.cpp | 2 +- src/App/DynamicProperty.cpp | 2 +- src/App/GeoFeaturePyImp.cpp | 4 ++-- src/App/Link.cpp | 2 +- src/App/ObjectIdentifier.cpp | 4 +++- src/App/Property.cpp | 14 ++++++++++++-- src/App/Property.h | 14 +++++++++++++- src/App/PropertyLinks.cpp | 2 +- src/Gui/DAGView/DAGModel.cpp | 2 +- src/Gui/SelectionView.cpp | 2 +- src/Mod/Part/App/PartFeature.cpp | 2 +- src/Mod/PartDesign/Gui/TaskFeaturePick.cpp | 2 +- src/Mod/PartDesign/Gui/Utils.cpp | 2 +- src/Mod/Spreadsheet/Gui/DlgSheetConf.cpp | 2 +- 14 files changed, 40 insertions(+), 16 deletions(-) diff --git a/src/App/DocumentObserver.cpp b/src/App/DocumentObserver.cpp index 2af08de2f15a..2aa04a1aa135 100644 --- a/src/App/DocumentObserver.cpp +++ b/src/App/DocumentObserver.cpp @@ -180,7 +180,7 @@ void DocumentObjectT::operator=(const DocumentObject* obj) } void DocumentObjectT::operator=(const Property *prop) { - if(!prop || !prop->getName() + if(!prop || !prop->hasName() || !prop->getContainer() || !prop->getContainer()->isDerivedFrom(App::DocumentObject::getClassTypeId())) { diff --git a/src/App/DynamicProperty.cpp b/src/App/DynamicProperty.cpp index 325fc8af68dd..a5c83b3e8030 100644 --- a/src/App/DynamicProperty.cpp +++ b/src/App/DynamicProperty.cpp @@ -211,7 +211,7 @@ Property* DynamicProperty::addDynamicProperty(PropertyContainer &pc, const char* bool DynamicProperty::addProperty(Property *prop) { - if(!prop || !prop->getName()) + if(!prop || !prop->hasName()) return false; auto &index = props.get<0>(); if(index.count(prop->getName())) diff --git a/src/App/GeoFeaturePyImp.cpp b/src/App/GeoFeaturePyImp.cpp index 27a0fb30b75a..d2bd0941aad6 100644 --- a/src/App/GeoFeaturePyImp.cpp +++ b/src/App/GeoFeaturePyImp.cpp @@ -66,8 +66,8 @@ PyObject* GeoFeaturePy::getPropertyNameOfGeometry(PyObject * args) GeoFeature* object = this->getGeoFeaturePtr(); const PropertyComplexGeoData* prop = object->getPropertyOfGeometry(); - const char* name = prop ? prop->getName() : 0; - if (name) { + const char* name = prop ? prop->getName() : nullptr; + if (Property::isValidName(name)) { return Py::new_reference_to(Py::String(std::string(name))); } return Py::new_reference_to(Py::None()); diff --git a/src/App/Link.cpp b/src/App/Link.cpp index c845d3b2474c..a8229875a6ae 100644 --- a/src/App/Link.cpp +++ b/src/App/Link.cpp @@ -184,7 +184,7 @@ void LinkBaseExtension::setProperty(int idx, Property *prop) { propName = prop->getName(); else propName = extensionGetPropertyName(prop); - if(!propName) + if(!Property::isValidName(propName)) propName = "?"; FC_TRACE("set property " << infos[idx].name << ": " << propName); } diff --git a/src/App/ObjectIdentifier.cpp b/src/App/ObjectIdentifier.cpp index e8e02e629910..31bfd219533c 100644 --- a/src/App/ObjectIdentifier.cpp +++ b/src/App/ObjectIdentifier.cpp @@ -165,7 +165,9 @@ ObjectIdentifier::ObjectIdentifier(const Property &prop, int index) DocumentObject * docObj = freecad_dynamic_cast(prop.getContainer()); if (!docObj) - FC_THROWM(Base::TypeError,"Property must be owned by a document object."); + FC_THROWM(Base::TypeError, "Property must be owned by a document object."); + if (!prop.hasName()) + FC_THROWM(Base::RuntimeError, "Property must have a name."); owner = const_cast(docObj); diff --git a/src/App/Property.cpp b/src/App/Property.cpp index 2c97b9d22f4c..2e7fafe282f0 100644 --- a/src/App/Property.cpp +++ b/src/App/Property.cpp @@ -61,9 +61,19 @@ Property::~Property() } -const char* Property::getName(void) const +const char* Property::getName() const { - return myName; + return myName ? myName : ""; +} + +bool Property::hasName() const +{ + return isValidName(myName); +} + +bool Property::isValidName(const char* name) +{ + return name && name[0] != '\0'; } std::string Property::getFullName() const { diff --git a/src/App/Property.h b/src/App/Property.h index 88d9deb7480c..9a7732542fa4 100644 --- a/src/App/Property.h +++ b/src/App/Property.h @@ -118,8 +118,20 @@ class AppExport Property : public Base::Persistence return sizeof(father) + sizeof(StatusBits); } - /// get the name of this property in the belonging container + /** Get the name of this property in the belonging container + * With \ref hasName() it can be checked beforehand if a valid name is set. + * @note If no name is set this function returns an empty string, i.e. "". + */ const char* getName(void) const; + /** Check if the property has a name set. + * If no name is set then \ref getName() will return an empty string + */ + bool hasName() const; + /** Check if the passed name is valid. + * If \a name is null or an empty string it's considered invalid, + * and valid otherwise. + */ + static bool isValidName(const char* name); std::string getFullName() const; diff --git a/src/App/PropertyLinks.cpp b/src/App/PropertyLinks.cpp index 1850f610aedc..3144bb3b3db8 100644 --- a/src/App/PropertyLinks.cpp +++ b/src/App/PropertyLinks.cpp @@ -204,7 +204,7 @@ PropertyLinkBase::updateLabelReferences(App::DocumentObject *obj, const char *ne static std::string propertyName(const Property *prop) { if(!prop) return std::string(); - if(!prop->getContainer() || !prop->getName()) { + if(!prop->getContainer() || !prop->hasName()) { auto xlink = Base::freecad_dynamic_cast(prop); if(xlink) return propertyName(xlink->parent()); diff --git a/src/Gui/DAGView/DAGModel.cpp b/src/Gui/DAGView/DAGModel.cpp index 49521a8b23ac..42c76919fbf8 100644 --- a/src/Gui/DAGView/DAGModel.cpp +++ b/src/Gui/DAGView/DAGModel.cpp @@ -302,7 +302,7 @@ void Model::slotDeleteObject(const ViewProviderDocumentObject &VPDObjectIn) void Model::slotChangeObject(const ViewProviderDocumentObject &VPDObjectIn, const App::Property& propertyIn) { std::string name("Empty Name"); - if (propertyIn.getName()) //getName can return 0. + if (propertyIn.hasName()) name = propertyIn.getName(); assert(!name.empty()); diff --git a/src/Gui/SelectionView.cpp b/src/Gui/SelectionView.cpp index 0d7ec7911438..7fb56f8e76e4 100644 --- a/src/Gui/SelectionView.cpp +++ b/src/Gui/SelectionView.cpp @@ -551,7 +551,7 @@ QString SelectionView::getProperty(App::DocumentObject* obj) const App::GeoFeature* geo = static_cast(obj); const App::PropertyComplexGeoData* data = geo->getPropertyOfGeometry(); const char* name = data ? data->getName() : nullptr; - if (name) { + if (App::Property::isValidName(name)) { property = QString::fromLatin1(name); } } diff --git a/src/Mod/Part/App/PartFeature.cpp b/src/Mod/Part/App/PartFeature.cpp index 252a2d089140..a16d0d09f317 100644 --- a/src/Mod/Part/App/PartFeature.cpp +++ b/src/Mod/Part/App/PartFeature.cpp @@ -236,7 +236,7 @@ struct ShapeCache { void slotChanged(const App::DocumentObject &obj, const App::Property &prop) { const char *propName = prop.getName(); - if(!propName) + if(!App::Property::isValidName(propName)) return; if(strcmp(propName,"Shape")==0 || strcmp(propName,"Group")==0 diff --git a/src/Mod/PartDesign/Gui/TaskFeaturePick.cpp b/src/Mod/PartDesign/Gui/TaskFeaturePick.cpp index 905323e59b9b..b350a300d946 100644 --- a/src/Mod/PartDesign/Gui/TaskFeaturePick.cpp +++ b/src/Mod/PartDesign/Gui/TaskFeaturePick.cpp @@ -340,7 +340,7 @@ App::DocumentObject* TaskFeaturePick::makeCopy(App::DocumentObject* obj, std::st App::Property* cprop = *it++; - if( strcmp(prop->getName(), "Label") == 0 ) { + if( prop->getName() && strcmp(prop->getName(), "Label") == 0 ) { static_cast(cprop)->setValue(name.c_str()); continue; } diff --git a/src/Mod/PartDesign/Gui/Utils.cpp b/src/Mod/PartDesign/Gui/Utils.cpp index e393aa6f6f12..d35838de9491 100644 --- a/src/Mod/PartDesign/Gui/Utils.cpp +++ b/src/Mod/PartDesign/Gui/Utils.cpp @@ -435,7 +435,7 @@ void relinkToBody (PartDesign::Feature *feature) { } } - if ( !valueStr.empty () ) { + if ( !valueStr.empty () && prop->hasName()) { FCMD_OBJ_CMD(obj,prop->getName() << '=' << valueStr); } } diff --git a/src/Mod/Spreadsheet/Gui/DlgSheetConf.cpp b/src/Mod/Spreadsheet/Gui/DlgSheetConf.cpp index cc40d9b60467..352a0e39afd2 100644 --- a/src/Mod/Spreadsheet/Gui/DlgSheetConf.cpp +++ b/src/Mod/Spreadsheet/Gui/DlgSheetConf.cpp @@ -141,7 +141,7 @@ App::Property *DlgSheetConf::prepare(CellAddress &from, CellAddress &to, vexpr->getPath().getProperty()); if(prop) { auto obj = Base::freecad_dynamic_cast(prop->getContainer()); - if(obj) { + if (obj && prop->hasName()) { path = ObjectIdentifier(sheet); path.setDocumentObjectName(obj,true); path << ObjectIdentifier::SimpleComponent(prop->getName());