Changeset 033646

Timestamp:

Dec 26, 2025, 9:40:14 PM (4 days ago)

Author:

Frederik Heber <frederik.heber@…>

Branches:

Candidate_v1.7.1, stable

Children:

c742bb1

Parents:

c8cb0d

git-author:

Frederik Heber <frederik.heber@…> (12/10/25 21:00:56)

git-committer:

Frederik Heber <frederik.heber@…> (12/26/25 21:40:14)

Message:

FIX: Insecure access in GLWorldScene on reparenting.

• We dereferenced two nested pointers where the first access was checked but the second not. This is now done cleanly in the reparenting in GLWorldScene. ... except for the last getIndex() access. But at least in may only be already free'd memory and not NULL.

File:

• src/UIElements/Views/Qt4/Qt3D/GLWorldScene.cpp (modified) (3 diffs)

src/UIElements/Views/Qt4/Qt3D/GLWorldScene.cpp

@@ -421 +421 @@
   }
   {
-    if ((_bond->getLeftAtom() != NULL) && (_bond->getLeftAtom()->getMoleculeRef() != NULL))
-      BondNodeParentMaps[0].left.insert(
-          std::make_pair(bondid, _bond->getLeftAtom()->getMoleculeRef()->getIndex()) );
-    else
-      BondNodeParentMaps[0].left.insert(
-          std::make_pair(bondid, (ObservedValue_Index_t)NULL) );
-    if ((_bond->getRightAtom() != NULL) && (_bond->getRightAtom()->getMoleculeRef() != NULL))
-      BondNodeParentMaps[1].left.insert(
-          std::make_pair(bondid, _bond->getRightAtom()->getMoleculeRef()->getIndex()) );
-    else
-      BondNodeParentMaps[1].left.insert(
-          std::make_pair(bondid, (ObservedValue_Index_t)NULL) );
+    {
+      const QtObservedAtom::ptr leftAtom = _bond->getLeftAtom();
+      if (leftAtom != NULL) {
+        const QtObservedMolecule * const molRef = leftAtom->getMoleculeRef();
+        if (molRef != NULL)
+          BondNodeParentMaps[0].left.insert(
+              std::make_pair(bondid, molRef->getIndex()) );
+        else
+          BondNodeParentMaps[0].left.insert(
+              std::make_pair(bondid, (ObservedValue_Index_t)NULL) );
+      } else {
+        BondNodeParentMaps[0].left.insert(
+            std::make_pair(bondid, (ObservedValue_Index_t)NULL) );
+      }
+    }
+    {
+      const QtObservedAtom::ptr rightAtom = _bond->getRightAtom();
+      if (rightAtom != NULL) {
+        const QtObservedMolecule * const molRef = rightAtom->getMoleculeRef();
+        if (molRef != NULL)
+          BondNodeParentMaps[1].left.insert(
+              std::make_pair(bondid, molRef->getIndex()) );
+        else
+          BondNodeParentMaps[1].left.insert(
+              std::make_pair(bondid, (ObservedValue_Index_t)NULL) );
+      } else {
+        BondNodeParentMaps[1].left.insert(
+            std::make_pair(bondid, (ObservedValue_Index_t)NULL) );
+      }
+    }
   }
 
@@ -744 +762 @@
   // change parent entry
   AtomNodeParentMap.left.erase(parentiter);
-  if (walker->getMoleculeRef() != NULL)
-    AtomNodeParentMap.left.insert( std::make_pair(walkerid, walker->getMoleculeRef()->getIndex()) );
+  const QtObservedMolecule * molRef = walker->getMoleculeRef();
+  if (molRef != NULL)
+    AtomNodeParentMap.left.insert( std::make_pair(walkerid, molRef->getIndex()) );
   else
     AtomNodeParentMap.left.insert( std::make_pair(walkerid, (ObservedValue_Index_t)NULL) );
@@ -841 +860 @@
   // change parent entry
   BondNodeParentMaps[dim].left.erase(bondid);
-  if ((_atom != NULL) && (_atom->getMoleculeRef() != NULL))
-    BondNodeParentMaps[dim].left.insert( std::make_pair( bondid, _atom->getMoleculeRef()->getIndex()));
-  else
+  if (_atom != NULL) {
+    const QtObservedMolecule * const molRef = _atom->getMoleculeRef();
+    if (molRef != NULL)
+      BondNodeParentMaps[dim].left.insert( std::make_pair( bondid, molRef->getIndex()));
+    else
+      BondNodeParentMaps[dim].left.insert( std::make_pair( bondid, (ObservedValue_Index_t)NULL) );
+  } else {
     BondNodeParentMaps[dim].left.insert( std::make_pair( bondid, (ObservedValue_Index_t)NULL) );
+  }
   parentiter = BondNodeParentMaps[dim].left.find(bondid);
   LOG(3, "INFO: GLWorldScene::reparentBond() - Reparented bond "