Fix BGF overflow reported by @rphang

Potential heap-buffer-overflow at bgfformat.cpp:172
CONECT 3 in a 2-atom file previously accepted
Now checks against NumAtoms() since it's unsigned

Signed-off-by: Geoff Hutchison <geoff.hutchison@gmail.com>

src/formats/bgfformat.cpp

@@ -164,8 +164,12 @@ namespace OpenBabel
 
         if (EQn(buffer,"CONECT",6))
           {
+            // bgn is unsigned: a zero or negative index in the file wraps
+            // to a huge value and is caught by the same bound. The former
+            // "bgn < 1" test wrongly rejected bgn == 0, silently discarding
+            // the CONECT record of the *first* atom.
             bgn = atoi((char*)vs[1].c_str()) - 1;
-            if (bgn < 1 || bgn > mol.NumAtoms())
+            if (bgn >= mol.NumAtoms())
               continue;
             for (i = 2;i < vs.size();i++)
               {
@@ -176,8 +180,9 @@ namespace OpenBabel
         else
           if (EQn(buffer,"ORDER",5))
             {
+              // See the CONECT branch above regarding this bound.
               bgn = atoi((char*)vs[1].c_str()) - 1;
-              if (bgn < 1 || bgn > mol.NumAtoms())
+              if (bgn >= mol.NumAtoms())
                 continue;
               if (vs.size() > vord[bgn].size()+2)
                 continue;

test/CMakeLists.txt

@@ -39,7 +39,7 @@ set(carspacegroup_parts 1 2 3 4)
 set(cifspacegroup_parts 1 2 3 4 5 6 7 8 9 10 11 12 13)
 set(cistrans_parts 1 2 3 4 5 6 7 8 9)
 set(conversion_parts 1)
-set(fuzzregress_parts 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43)
+set(fuzzregress_parts 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 48 49)
 set(graphsym_parts 1 2 3 4 5)
 set(gzip_parts 1)
 set(addh_parts 1)

test/files/fuzz_regress/bgf-atom1-conect.bgf

@@ -0,0 +1,8 @@
+BIOGRF  332
+DESCRP atom1-only
+FORMAT ATOM   (a6,1x,i5,1x,a5,1x,a3,1x,a1,1x,a5,3f10.5,1x,a5,i3,i2,1x,f8.5)
+ATOM       1 C1   RES  A  444    0.00000   0.00000   0.00000 C_3    1 1  0.00000
+ATOM       2 C2   RES  A  444    1.50000   0.00000   0.00000 C_3    1 1  0.00000
+FORMAT CONECT (a6,12i6)
+CONECT     1     2
+END

test/files/fuzz_regress/bgf-conect-oob.bgf

@@ -0,0 +1,8 @@
+BIOGRF  332
+DESCRP oob
+FORMAT ATOM   (a6,1x,i5,1x,a5,1x,a3,1x,a1,1x,a5,3f10.5,1x,a5,i3,i2,1x,f8.5)
+ATOM       1 C1   RES  A  444    0.00000   0.00000   0.00000 C_3    1 1  0.00000
+ATOM       2 C2   RES  A  444    1.50000   0.00000   0.00000 C_3    1 1  0.00000
+FORMAT CONECT (a6,12i6)
+CONECT     3     1     2
+END

test/fuzzregresstest.cpp

@@ -169,6 +169,44 @@ static bool RunReproExpectReject(const string &caseId, const string &inFormat,
   return true;
 }
 
+// Read a reproducer that is *well-formed* and require that it still parses
+// into the expected molecule. Used to pin the accept-side behaviour of a
+// hardening fix: bounds checks that are too aggressive would silently drop
+// good atoms/bonds, which no sanitizer and no "must not crash" test can see.
+// Skip silently if the corpus file or format is unavailable.
+static bool RunReproExpectMolecule(const string &caseId, const string &inFormat,
+                                   const string &filename,
+                                   unsigned int expectedAtoms,
+                                   unsigned int expectedBonds)
+{
+  string path = GetFuzzFile(filename);
+  ifstream probe(path.c_str());
+  if (!probe.good()) {
+    cout << "# skip " << caseId << ": corpus file missing (" << path << ")\n";
+    return true;
+  }
+
+  OBConversion conv;
+  if (!conv.SetInFormat(inFormat.c_str())) {
+    cout << "# skip " << caseId << ": format " << inFormat
+         << " not registered in this build\n";
+    return true;
+  }
+
+  OBMol mol;
+  if (!conv.ReadFile(&mol, path)) {
+    cout << "# FAIL " << caseId << ": valid file was rejected\n";
+    return false;
+  }
+  if (mol.NumAtoms() != expectedAtoms || mol.NumBonds() != expectedBonds) {
+    cout << "# FAIL " << caseId << ": expected " << expectedAtoms << " atoms / "
+         << expectedBonds << " bonds, got " << mol.NumAtoms() << " / "
+         << mol.NumBonds() << "\n";
+    return false;
+  }
+  return true;
+}
+
 // CVE-2026-2704: heap-buffer-overflow in transform3d::DescribeAsString
 // when parsing a CIF with an all-zero row in a space-group transform.
 // Fixed in PR #2862.
@@ -642,6 +680,35 @@ void caseMcdlTruncatedCycle()
                             "mcdl-truncated-cycle.smi"));
 }
 
+// BGF out-of-bounds CONECT/ORDER index (no CVE id): heap-buffer-overflow write
+// in BGFFormat::ReadMolecule. vcon/vord are sized to exactly NumAtoms entries,
+// but the CONECT handler converted the file's 1-based atom index to 0-based
+// (bgn = atoi(..) - 1) and then bounds-checked it with "bgn > mol.NumAtoms()".
+// An index of NumAtoms+1 yields bgn == NumAtoms, which that test accepts, so
+// vcon[bgn].push_back() ran a phantom std::vector control block read out of
+// adjacent heap memory and wrote through it. The ORDER branch repeated the
+// same guard and additionally did an arbitrary write, vord[bgn][i-2] = atoi().
+// Fixed by testing "bgn >= mol.NumAtoms()" at both sites; bgn is unsigned, so
+// zero/negative file indices wrap and are caught by the same bound.
+void caseBgfConectOob()
+{
+  OB_ASSERT(RunRepro("bgf-conect-oob", "bgf", "bgf-conect-oob.bgf"));
+}
+
+// BGF first-atom CONECT record (no CVE id): the same guard's lower half,
+// "bgn < 1", rejected bgn == 0 -- which is the *first* atom, not an invalid
+// index -- so every CONECT/ORDER record belonging to atom 1 was silently
+// discarded. BGF normally lists connectivity symmetrically, so the partner's
+// record usually re-added the bond and masked the loss; it only shows up when
+// a bond is listed solely on atom 1, as here. This is an accept-side pin for
+// the memory-safety fix above: the file must read as 2 atoms and 1 bond.
+// Before the fix it read as 2 atoms and 0 bonds.
+void caseBgfAtom1Conect()
+{
+  OB_ASSERT(RunReproExpectMolecule("bgf-atom1-conect", "bgf",
+                                   "bgf-atom1-conect.bgf", 2, 1));
+}
+
 int fuzzregresstest(int argc, char *argv[])
 {
   int defaultchoice = 1;
@@ -790,6 +857,12 @@ int fuzzregresstest(int argc, char *argv[])
   case 43:
     caseMcdlTruncatedCycle();
     break;
+  case 48:
+    caseBgfConectOob();
+    break;
+  case 49:
+    caseBgfAtom1Conect();
+    break;
   default:
     cout << "Test number " << choice << " does not exist!\n";
     return -1;