Prevent out of boundaries access in packmol.f90

[afonari]

While reading input options.

[lmiq]

Thanks for the contributions. Having a safeguard for that is nice, but we should throw an informative error instead, guiding to user to what to do (in this case, increase the maxkeywords parameter or revise the input file for errors, more likely).

[afonari]

From what I understand, the input can actually be correct, since number of values could be maxkeywords.
This is similar to the original condition (which is not an input error):

            if ( ioerr /= 0 ) exit

But doesn't invoke possible out of boundaries call.

if ( keyword(iline,1) == "radius" ) then
          read(keyword(iline,2),*,iostat=ioerr) value
          if ( ioerr /= 0 ) then
            write(*,*) ' ERROR: Could not read radius from keyword. '
            stop exit_code_input_error
          end if
          ival = 2
          do
            if (ival > maxkeywords) exit ! <-- This is new line
            read(keyword(iline_atoms,ival),*,iostat=ioerr) iat
            if ( ioerr /= 0 ) exit ! This is original exit, which is not an input error I think
            if ( iat > natoms(itype) ) then
              write(*,*) ' ERROR: atom selection with index greater than number of '
              write(*,*) '        atoms in structure ', itype
              stop exit_code_input_error
            end if
            radius(icart+iat) = value
            ival = ival + 1
          end do
        end if

[lmiq]

Oh, no, not really.

These properties are not bound to maxkeywords values, because the values read are stored in a specific array of atom properties, one for each atom (the radius of each type of atom). Thus, the number of values can indeed be greater than maxkeywords.

The exit on reading error is used there to read until the end of line.

[afonari]

Say I have:

...
structure file.xyz
  atoms 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20
    radius 2.35
  end atoms
number 1
end structure
...

keyword shape is: 14 21

When ival is 21, read value is 20 (this is correct). Next iteration the call is:

read(keyword(iline_atoms,ival),*,iostat=ioerr) iat

where ival is 22, out of bounds happens (keyword(iline_atoms, 22)) before read function is called. Maybe I'm missing something.

[lmiq]

Do you have an example of that happening? Because maxkeywords is a dynamically set value (setsizes.f90 - line 105), and it is set to the maximum number of keywords found, such that the further reading of the file should never exceed that.

[afonari]

Attaching example:
example.tar.gz

Here are the debug print:

           end if
           ival = 2
           do
+            IF ( ival > maxkeywords ) THEN
+              write(*,*), 'Here', ival, maxkeywords, iat, shape(keyword)
+            END IF
             read(keyword(iline_atoms,ival),*,iostat=ioerr) iat
+            IF ( ival > maxkeywords ) THEN
+              write(*,*), 'Here1', ival, maxkeywords, iat, shape(keyword), ioerr
+            END IF
             if ( ioerr /= 0 ) exit
             if ( iat > natoms(itype) ) then
               write(*,*) ' ERROR: atom selection with index greater than number of '

Output:

Number of molecules of type            1 :            1
  Total number of restrictions:            0
  Total number of atoms:           35
  Total number of molecules:            1
  Number of fixed molecules:            0
  Number of free molecules:            1
  Number of variables:            6
  Total number of fixed atoms:            0
 Here          22          21          20          14          21
 Here1          22          21          20          14          21          59
  PBC on: Constraint automatically added to non-fixed atoms:
  -> inside box     0.00    0.00    0.00   11.00   11.00   11.00

[lmiq]

Thanks, I had now time to look at it, and indeed, that was needed there. I changed in the main code your suggestion to the use of "do while", though. But thank you for pointing the issue. The fix is available in the 21.2.1 release.