fix(lammps): validate dplr and spin options

source/lmp/fix_dplr.cpp

@@ -80,16 +80,21 @@ FixDPLR::FixDPLR(LAMMPS* lmp, int narg, char** arg)
       error->all(FLERR, "Illegal fix command\nwrong number of parameters\n");
     }
     if (string(arg[iarg]) == string("model")) {
-      if (iarg + 1 > narg) {
-        error->all(FLERR, "Illegal fix adapt command");
+      if (iarg + 1 >= narg || is_key(arg[iarg + 1])) {
+        error->all(FLERR, "Illegal fix dplr model option, not provided");
       }
       model = string(arg[iarg + 1]);
       iarg += 2;
     } else if (string(arg[iarg]) == string("efield")) {
-      if (iarg + 3 > narg) {
-        error->all(FLERR,
-                   "Illegal fix adapt command, efield should be provided 3 "
-                   "float numbers");
+      // A following option keyword is not an electric-field component. Check
+      // all three positions before reading any of them so truncated commands
+      // cannot access beyond LAMMPS's argument array.
+      for (int ii = 1; ii <= 3; ++ii) {
+        if (iarg + ii >= narg || is_key(arg[iarg + ii])) {
+          error->all(FLERR,
+                     "Illegal fix dplr efield option, three values are "
+                     "required");
+        }
       }
       if (utils::strmatch(arg[iarg + 1], "^v_")) {
         xstr = utils::strdup(arg[iarg + 1] + 2);
@@ -137,8 +142,12 @@ FixDPLR::FixDPLR(LAMMPS* lmp, int narg, char** arg)
       break;
     }
   }
-  assert(map_vec.size() % 2 == 0 &&
-         "number of ints provided by type_associate should be even");
+  if (map_vec.size() % 2 != 0) {
+    error->all(
+        FLERR,
+        "Illegal fix dplr type_associate option, an even number of atom types "
+        "is required");
+  }
 
   // dpt.init(model);
   // dtm.init("frozen_model.pb");

source/lmp/pair_deepmd.cpp

@@ -25,6 +25,7 @@
 #include "neighbor.h"
 #include "output.h"
 #include "update.h"
+#include "utils.h"
 #if LAMMPS_VERSION_NUMBER >= 20210831
 // in lammps #2902, fix_ttm members turns from private to protected
 #define USE_TTM 1
@@ -837,23 +838,43 @@ void PairDeepMD::settings(int narg, char** arg) {
       out_each = 1;
       iarg += 1;
     } else if (string(arg[iarg]) == string("relative")) {
+      if (iarg + 1 >= narg || is_key(arg[iarg + 1])) {
+        error->all(FLERR, "Illegal relative, not provided");
+      }
       out_rel = 1;
-      eps = atof(arg[iarg + 1]) / ener_unit_cvt_factor;
+      eps = utils::numeric(FLERR, arg[iarg + 1], false, lmp) /
+            ener_unit_cvt_factor;
       iarg += 2;
     } else if (string(arg[iarg]) == string("relative_v")) {
+      if (iarg + 1 >= narg || is_key(arg[iarg + 1])) {
+        error->all(FLERR, "Illegal relative_v, not provided");
+      }
       out_rel_v = 1;
-      eps_v = atof(arg[iarg + 1]) / ener_unit_cvt_factor;
+      eps_v = utils::numeric(FLERR, arg[iarg + 1], false, lmp) /
+              ener_unit_cvt_factor;
       iarg += 2;
     } else if (string(arg[iarg]) == string("virtual_len")) {
       virtual_len.resize(numb_types_spin);
       for (int ii = 0; ii < numb_types_spin; ++ii) {
-        virtual_len[ii] = atof(arg[iarg + ii + 1]);
+        if (iarg + ii + 1 >= narg || is_key(arg[iarg + ii + 1])) {
+          char tmp[1024];
+          sprintf(tmp, "Illegal virtual_len, the dimension should be %d",
+                  numb_types_spin);
+          error->all(FLERR, tmp);
+        }
+        virtual_len[ii] = utils::numeric(FLERR, arg[iarg + ii + 1], false, lmp);
       }
       iarg += numb_types_spin + 1;
     } else if (string(arg[iarg]) == string("spin_norm")) {
       spin_norm.resize(numb_types_spin);
       for (int ii = 0; ii < numb_types_spin; ++ii) {
-        spin_norm[ii] = atof(arg[iarg + ii + 1]);
+        if (iarg + ii + 1 >= narg || is_key(arg[iarg + ii + 1])) {
+          char tmp[1024];
+          sprintf(tmp, "Illegal spin_norm, the dimension should be %d",
+                  numb_types_spin);
+          error->all(FLERR, tmp);
+        }
+        spin_norm[ii] = utils::numeric(FLERR, arg[iarg + ii + 1], false, lmp);
       }
       iarg += numb_types_spin + 1;
     }

source/lmp/pair_deepspin.cpp

@@ -23,6 +23,7 @@
 #include "neighbor.h"
 #include "output.h"
 #include "update.h"
+#include "utils.h"
 #if LAMMPS_VERSION_NUMBER >= 20210831
 // in lammps #2902, fix_ttm members turns from private to protected
 #define USE_TTM 1
@@ -719,23 +720,43 @@ void PairDeepSpin::settings(int narg, char** arg) {
       out_each = 1;
       iarg += 1;
     } else if (string(arg[iarg]) == string("relative")) {
+      if (iarg + 1 >= narg || is_key(arg[iarg + 1])) {
+        error->all(FLERR, "Illegal relative, not provided");
+      }
       out_rel = 1;
-      eps = atof(arg[iarg + 1]) / ener_unit_cvt_factor;
+      eps = utils::numeric(FLERR, arg[iarg + 1], false, lmp) /
+            ener_unit_cvt_factor;
       iarg += 2;
     } else if (string(arg[iarg]) == string("relative_v")) {
+      if (iarg + 1 >= narg || is_key(arg[iarg + 1])) {
+        error->all(FLERR, "Illegal relative_v, not provided");
+      }
       out_rel_v = 1;
-      eps_v = atof(arg[iarg + 1]) / ener_unit_cvt_factor;
+      eps_v = utils::numeric(FLERR, arg[iarg + 1], false, lmp) /
+              ener_unit_cvt_factor;
       iarg += 2;
     } else if (string(arg[iarg]) == string("virtual_len")) {
       virtual_len.resize(numb_types_spin);
       for (int ii = 0; ii < numb_types_spin; ++ii) {
-        virtual_len[ii] = atof(arg[iarg + ii + 1]);
+        if (iarg + ii + 1 >= narg || is_key(arg[iarg + ii + 1])) {
+          char tmp[1024];
+          sprintf(tmp, "Illegal virtual_len, the dimension should be %d",
+                  numb_types_spin);
+          error->all(FLERR, tmp);
+        }
+        virtual_len[ii] = utils::numeric(FLERR, arg[iarg + ii + 1], false, lmp);
       }
       iarg += numb_types_spin + 1;
     } else if (string(arg[iarg]) == string("spin_norm")) {
       spin_norm.resize(numb_types_spin);
       for (int ii = 0; ii < numb_types_spin; ++ii) {
-        spin_norm[ii] = atof(arg[iarg + ii + 1]);
+        if (iarg + ii + 1 >= narg || is_key(arg[iarg + ii + 1])) {
+          char tmp[1024];
+          sprintf(tmp, "Illegal spin_norm, the dimension should be %d",
+                  numb_types_spin);
+          error->all(FLERR, tmp);
+        }
+        spin_norm[ii] = utils::numeric(FLERR, arg[iarg + ii + 1], false, lmp);
       }
       iarg += numb_types_spin + 1;
     }

source/lmp/tests/test_lammps_option_parsers.py

@@ -0,0 +1,182 @@
+# SPDX-License-Identifier: LGPL-3.0-or-later
+"""Regression tests for malformed DeePMD LAMMPS option lists.
+
+The existing LAMMPS tests exercise complete, valid option lists. These cases
+run out of process so a future out-of-bounds parser regression fails one test
+without crashing the entire pytest worker.
+"""
+
+import os
+import subprocess as sp
+import sys
+from pathlib import (
+    Path,
+)
+
+import pytest
+from lammps_test_utils import (
+    require_backend,
+)
+from model_convert import (
+    ensure_converted_pb,
+)
+
+pbtxt_file = (
+    Path(__file__).parent.parent.parent / "tests" / "infer" / "deepspin_nlist.pbtxt"
+)
+
+_LAMMPS_RUNNER = """
+import os
+import sys
+
+from lammps import lammps
+
+lmp = lammps(cmdargs=["-log", "none", "-screen", "none"])
+try:
+    if plugin := os.environ.get("DEEPMD_TEST_PLUGIN"):
+        lmp.command(f"plugin load {plugin}")
+    for command in sys.argv[1:]:
+        lmp.command(command)
+finally:
+    lmp.close()
+"""
+
+
+def setup_module() -> None:
+    require_backend("ENABLE_TENSORFLOW", "TensorFlow")
+
+
+@pytest.fixture(scope="module")
+def spin_model(tmp_path_factory: pytest.TempPathFactory) -> Path:
+    """Build a one-spin-type model used to size spin option value lists."""
+    model = tmp_path_factory.mktemp("lammps_parser") / "deepspin.pb"
+    ensure_converted_pb(pbtxt_file, model)
+    return model
+
+
+def _run_lammps(*commands: str) -> sp.CompletedProcess[str]:
+    """Isolate malformed commands that could crash an unfixed LAMMPS parser."""
+    return sp.run(
+        [sys.executable, "-c", _LAMMPS_RUNNER, *commands],
+        capture_output=True,
+        text=True,
+        timeout=60,
+        check=False,
+        env=os.environ.copy(),
+    )
+
+
+def _assert_lammps_error(result: sp.CompletedProcess[str], message: str) -> None:
+    """Require the controlled diagnostic, not merely a crashed subprocess."""
+    output = result.stdout + result.stderr
+    assert result.returncode != 0, output
+    assert f"ERROR: {message} (" in output, output
+
+
+@pytest.mark.parametrize(
+    ("style", "option", "message"),
+    [
+        ("deepmd", "relative", "Illegal relative, not provided"),
+        ("deepmd", "relative_v", "Illegal relative_v, not provided"),
+        ("deepspin", "relative", "Illegal relative, not provided"),
+        ("deepspin", "relative_v", "Illegal relative_v, not provided"),
+        (
+            "deepspin",
+            "virtual_len",
+            "Illegal virtual_len, the dimension should be 1",
+        ),
+        (
+            "deepspin",
+            "spin_norm",
+            "Illegal spin_norm, the dimension should be 1",
+        ),
+    ],
+)
+def test_pair_style_rejects_truncated_option(
+    spin_model: Path, style: str, option: str, message: str
+) -> None:
+    result = _run_lammps(
+        "units metal", f"pair_style {style} {spin_model.resolve()} {option}"
+    )
+    _assert_lammps_error(result, message)
+
+
+@pytest.mark.parametrize("style", ["deepmd", "deepspin"])
+@pytest.mark.parametrize(
+    ("option", "message"),
+    [
+        ("relative", "Illegal relative, not provided"),
+        ("relative_v", "Illegal relative_v, not provided"),
+    ],
+)
+def test_pair_style_rejects_keyword_as_relative_value(
+    spin_model: Path, style: str, option: str, message: str
+) -> None:
+    result = _run_lammps(
+        "units metal",
+        f"pair_style {style} {spin_model.resolve()} {option} atomic",
+    )
+    _assert_lammps_error(result, message)
+
+
+@pytest.mark.parametrize(
+    ("option", "message"),
+    [
+        ("virtual_len", "Illegal virtual_len, the dimension should be 1"),
+        ("spin_norm", "Illegal spin_norm, the dimension should be 1"),
+    ],
+)
+def test_deepspin_rejects_keyword_as_spin_value(
+    spin_model: Path, option: str, message: str
+) -> None:
+    result = _run_lammps(
+        "units metal",
+        f"pair_style deepspin {spin_model.resolve()} {option} atomic",
+    )
+    _assert_lammps_error(result, message)
+
+
+def test_deepspin_accepts_complete_option_values(spin_model: Path) -> None:
+    """Keep the valid combined option path covered alongside negative cases."""
+    result = _run_lammps(
+        "units metal",
+        f"pair_style deepspin {spin_model.resolve()} relative 1.0 relative_v 2.0 "
+        "virtual_len 0.4 spin_norm 1.2737 atomic",
+    )
+    assert result.returncode == 0, result.stdout + result.stderr
+
+
+@pytest.mark.parametrize(
+    ("fix_options", "message"),
+    [
+        ("model", "Illegal fix dplr model option, not provided"),
+        (
+            "model efield 0 0 0",
+            "Illegal fix dplr model option, not provided",
+        ),
+        (
+            "model {model} efield 0 0",
+            "Illegal fix dplr efield option, three values are required",
+        ),
+        (
+            "model {model} efield 0 bond_type 1",
+            "Illegal fix dplr efield option, three values are required",
+        ),
+        (
+            "model {model} type_associate 1",
+            "Illegal fix dplr type_associate option, an even number of atom "
+            "types is required",
+        ),
+    ],
+)
+def test_fix_dplr_rejects_malformed_options(
+    spin_model: Path, fix_options: str, message: str
+) -> None:
+    result = _run_lammps(
+        "units metal",
+        "atom_style full",
+        "region box block 0 1 0 1 0 1",
+        "create_box 1 box",
+        f"fix 0 all dplr {fix_options.format(model=spin_model.resolve())}",
+    )
+    _assert_lammps_error(result, message)