Colvars / colvars

Collective variables library for molecular simulation and analysis programs
http://colvars.github.io/
GNU Lesser General Public License v3.0
208 stars 57 forks source link

Inconsistent GETCWD in src/colvarbias_meta.cpp can cause underfined behavior #726

Open al42and opened 3 hours ago

al42and commented 3 hours ago

In src/colvarbias_meta.cpp, there's GETCWD(BUF, SIZE) macro used to wrap getcwd function to put current working dir into BUF. However, when std::filesystem is available, a different implementation is used, which does not change BUF but instead returns a temporary C-string: https://github.com/Colvars/colvars/blob/0e0ce447630e95f2cd0322b1699e7f60b526ef9f/src/colvarbias_meta.cpp#L25C1-L30C7

This is not ok with how GETCWD is later used:

    char *pwd = new char[3001];
    if (GETCWD(pwd, 3000) == nullptr) {
      if (pwd != nullptr) { //
        delete[] pwd;
      }
      return cvm::error("Error: cannot get the path of the current working directory.\n",
                        COLVARS_BUG_ERROR);
    }

    replica_list_file =
      (std::string(pwd)+std::string(PATHSEP)+
       this->name+"."+replica_id+".files.txt");

The conditional will never be false, but pwd will not be initialized; thus so std::string(pwd) later is UB.

giacomofiorin commented 3 hours ago

Thanks! Will fix it here and include it in the GROMACS 2024 bugfixes as well as the Colvars lib update for 2025.