Reject malformed AGG/BAGG headers before construction

ChangeLog

@@ -25,6 +25,9 @@
 - Corrected calculation of total number of actions for a player in `pygambit` (#938)
 - Corrected a regression in action graph games that left the internal data structure not fully initialised,
   leading to segmentation faults.
+- Corrected handling of malformed AGG/BAGG files: files with an invalid or degenerate header
+  (for example a wrong file type, or a header declaring zero players) are now rejected with a
+  `ValueError` instead of causing a segmentation fault while constructing the game.
 
 ### Changed
 - Added a new welcome/landing window on launching the GUI without a game.  This has the effect of

src/games/agg/agg.cc

@@ -152,6 +152,10 @@ std::shared_ptr<AGG> AGG::makeAGG(istream &in)
   if (!in.good()) {
     throw std::runtime_error("Error reading the number of function nodes");
   }
+  if (n <= 0 || S < 0 || P < 0) {
+    throw std::runtime_error("Error in game file: invalid AGG header (number of players, "
+                             "action nodes, or function nodes out of range)");
+  }
   stripComment(in);
 
   // enter sizes of action sets:

src/games/agg/bagg.cc

@@ -73,18 +73,22 @@ std::shared_ptr<BAGG> BAGG::makeBAGG(istream &in)
   in >> S;
   stripComment(in);
   in >> P;
+  if (!in || N <= 0 || S < 0 || P < 0) {
+    throw std::runtime_error("Error in game file: expected BAGG header with number of "
+                             "players, action nodes, and function nodes");
+  }
 
   stripComment(in);
   vector<int> numTypes(N);
 
   // input number of types for each player
   for (int i = 0; i < N; ++i) {
-    if (in.eof() || in.bad()) {
+    in >> numTypes[i];
+    if (!in) {
       throw std::runtime_error(
           "Error in game file: integer expected for the number of types for player " +
           std::to_string(i));
     }
-    in >> numTypes[i];
   }
 
   // input the type distributions
@@ -93,10 +97,10 @@ std::shared_ptr<BAGG> BAGG::makeBAGG(istream &in)
   for (int i = 0; i < N; ++i) {
     TDist.emplace_back(numTypes[i]);
     for (int j = 0; j < numTypes[i]; ++j) {
-      if (in.eof() || in.bad()) {
+      in >> TDist[i][j];
+      if (!in) {
         throw std::runtime_error("Error in game file: number expected for type distribution");
       }
-      in >> TDist[i][j];
     }
   }
 
@@ -105,12 +109,12 @@ std::shared_ptr<BAGG> BAGG::makeBAGG(istream &in)
   vector<vector<vector<int>>> typeActionSets(N);
   for (int i = 0; i < N; ++i) {
     for (int j = 0; j < numTypes[i]; ++j) {
-      if (in.eof() || in.bad()) {
+      int temp;
+      in >> temp;
+      if (!in || temp < 0) {
         throw std::runtime_error(
             "Error in game file: integer expected for size of type action set");
       }
-      int temp;
-      in >> temp;
       typeActionSets[i].emplace_back(temp);
     }
   }
@@ -120,10 +124,10 @@ std::shared_ptr<BAGG> BAGG::makeBAGG(istream &in)
   for (int i = 0; i < N; ++i) {
     for (int j = 0; j < numTypes[i]; ++j) {
       for (int &el : typeActionSets[i][j]) {
-        if (in.eof() || in.bad()) {
+        in >> el;
+        if (!in) {
           throw std::runtime_error("Error in game file: integer expected for type action set");
         }
-        in >> el;
       }
     }
   }

tests/test_io.py

@@ -49,6 +49,11 @@ def test_read_agg_invalid():
         gbt.read_agg(game_path)
 
 
+def test_read_agg_zero_header():
+    with pytest.raises(ValueError):
+        gbt.read_agg(io.StringIO("0 0 0\n"))
+
+
 def test_read_bagg():
     game_path = os.path.join("contrib", "games", "Bayesian-Coffee-3-2-2-3.bagg")
     game = gbt.read_bagg(game_path)
@@ -63,6 +68,11 @@ def test_read_bagg_invalid():
         gbt.read_bagg(game_path)
 
 
+def test_read_bagg_zero_header():
+    with pytest.raises(ValueError):
+        gbt.read_bagg(io.StringIO("0 0 0\n"))
+
+
 def test_read_gbt_invalid():
     game_path = os.path.join(
         "tests", "test_games", "2x2x2_nfg_from_local_max_cut_2_pure_1_mixed_eq.nfg"