Fixes two out-of-bounds bugs in the InChI wrapper (rdkit#8126)

e-kwsm · Dec 25, 2024 · e6a45a8 · e6a45a8
1 parent bebd2eb
commit e6a45a8
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 7 deletions.
diff --git a/External/INCHI-API/inchi.cpp b/External/INCHI-API/inchi.cpp
@@ -1840,11 +1840,14 @@ std::string MolToInchi(const ROMol &mol, ExtraInchiReturnValues &rv,
     }
 
     // convert tetrahedral chirality info to Stereo0D
-    if (atom->getChiralTag() != Atom::CHI_UNSPECIFIED ||
-        atom->hasProp("molParity")) {
-      // we ignore the molParity if the number of neighbors are below 3
+    if (atom->getChiralTag() == Atom::ChiralType::CHI_TETRAHEDRAL_CCW ||
+        atom->getChiralTag() == Atom::ChiralType::CHI_TETRAHEDRAL_CW) {
       atom->calcImplicitValence();
-      if (atom->getNumImplicitHs() + atom->getDegree() < 3) {
+      if (auto tval = atom->getTotalValence(); tval < 3 || tval > 4) {
+        BOOST_LOG(rdWarningLog)
+            << "tetrahedral chirality on atom with <3 or >4 neighbors will be ignored."
+            << std::endl;
+
         continue;
       }
       inchi_Stereo0D stereo0D;
@@ -1865,7 +1868,6 @@ std::string MolToInchi(const ROMol &mol, ExtraInchiReturnValues &rv,
       // std::cerr<<" at: "<<atom->getIdx();
       for (const auto &p : neighbors) {
         stereo0D.neighbor[nid++] = p.second;
-        // std::cerr<<" "<<p.second;
       }
       if (nid == 3) {
         // std::cerr<<" nid==3, reorder";
@@ -1964,6 +1966,13 @@ std::string MolToInchi(const ROMol &mol, ExtraInchiReturnValues &rv,
 
     // neighbor
     unsigned int idx = inchiAtoms[atomIndex1].num_bonds;
+    // The InChI code has a max number of neighbors allowed:
+    if (idx >= MAXVAL) {
+      BOOST_LOG(rdErrorLog)
+          << " atom " << atomIndex1 << " has too many bonds: " << idx
+          << ". The InChI library supports at most " << MAXVAL << std::endl;
+      return "";
+    }
     inchiAtoms[atomIndex1].neighbor[idx] = atomIndex2;
 
     // bond type

diff --git a/External/INCHI-API/test.cpp b/External/INCHI-API/test.cpp
@@ -909,17 +909,41 @@ void testGithub5311() {
     TEST_ASSERT(m);
     ExtraInchiReturnValues tmp;
     auto inchi = MolToInchi(*m, tmp);
-    std::cerr << "!!! " << inchi << std::endl;
     TEST_ASSERT(inchi == "InChI=1S/H3O2P/c1-3-2/h3H2,(H,1,2)");
     BOOST_LOG(rdInfoLog) << "done" << std::endl;
   }
 }
+
+void testGithub8123() {
+  BOOST_LOG(rdErrorLog) << "-------------------------------------" << std::endl;
+  BOOST_LOG(rdInfoLog) << "testing github #8123" << std::endl;
+
+  {
+    // fails due to the presence of a dative bond
+    auto m =
+        "Cc1cc(C)c(N2CCN(c3c(C)cc(C)cc3C)C2[Ru@OH28]2(Cl)(Cl)=Cc3cc([S@SP3](=O)(=O)N(C)C)ccc3O->2C(C)C)c(C)c1"_smiles;
+    TEST_ASSERT(m);
+    ExtraInchiReturnValues tmp;
+    auto inchi = MolToInchi(*m, tmp);
+    TEST_ASSERT(inchi.empty());
+  }
+  {
+    auto m =
+        "CC[C@H]1OC(=O)C[C@@H](O)[C@H](C)[C@@H](O[C@@H]2O[C@H](C)[C@@H](O)C(N(C)C)C2O)[C@@H](CC=O)C[C@@H](C)C(=O)/C=C/C(C)=C/[C@@H]1CO.CC[C@H]1OC(=O)C[C@@H](O)[C@H](C)[C@@H](O[C@@H]2O[C@H](C)[C@@H](O[C@H]3CC(C)(O)[C@@H](O)C(C)O3)C(N(C)C)C2O)[C@@H](CC=O)C[C@@H](C)C(=O)/C=C/C(C)=C/[C@@H]1CO[C@@H]1OC(C)[C@@H](O)[C@H](OC)C1OC.[3H][Y]([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])([3H])[3H]"_smiles;
+    TEST_ASSERT(m);
+    ExtraInchiReturnValues tmp;
+    auto inchi = MolToInchi(*m, tmp);
+    TEST_ASSERT(inchi == "");
+  }
+  BOOST_LOG(rdInfoLog) << "done" << std::endl;
+}
+
 //-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*
 //
 //-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*
 int main() {
   RDLog::InitLogs();
-
+  boost::logging::enable_logs("rdApp.info");
   testGithubIssue3();
   testGithubIssue8();
   testGithubIssue40();
@@ -938,4 +962,5 @@ int main() {
   test_clean_up_on_kekulization_error();
   testGithub6172();
   testGithub5311();
+  testGithub8123();
 }