Global scan provenance
This issue was found by the Codex global repository scan of non-unit-test files in Stardust0831/Multiwfn, pinned to commit d2b770a531eb7039153198d04520b5992f15c49d.
Problem
Basin generation deliberately stores non-attractor sentinel IDs in gridbas: 0 for unassigned grids and -1 for trajectories reaching an isolated-system box boundary.
|
if (ifcorerho==1) then !Restore to valence electron density |
|
write(*,*) "Restoring valence density..." |
|
cubmat(:,:,:)=cubmat(:,:,:)-corerhogrid(:,:,:) |
|
end if |
|
|
|
numunassign=count(gridbas(ixlow:ixup,iylow:iyup,izlow:izup)==0) |
|
write(*,"(' The number of unassigned grids:',i12)") numunassign |
|
numgotobound=count(gridbas(ixlow:ixup,iylow:iyup,izlow:izup)==-1) |
|
write(*,"(' The number of grids travelled to box boundary:',i12)") numgotobound |
|
!Test if encountered box boundary for isolated system |
|
if (ifPBC==0.and.(inowx==1.or.inowx==nx.or.inowy==1.or.inowy==ny.or.inowz==1.or.inowz==nz)) then |
|
do itrjgrid=1,ntrjgrid |
|
gridbas(trjgrid(1,itrjgrid),trjgrid(2,itrjgrid),trjgrid(3,itrjgrid))=-1 |
|
end do |
|
exit !Terminate trajectory |
Low-value-attractor elimination then uses the raw gridbas value to index atttypelist, whose declared lower bound is 1:
|
subroutine elimlowvalatt(threslowvalatt,imode) |
|
use defvar |
|
use basinintmod |
|
implicit real*8 (a-h,o-z) |
|
integer imode |
|
real*8 threslowvalatt |
|
integer highvalatt(numatt) |
|
integer att2att(-2:numatt) !Convert indices of old attractors to new ones |
|
integer atttypelist(numatt) |
|
|
|
if (imode==1) then |
|
return |
|
else |
|
icounthigh=0 |
|
atttypelist=0 |
|
do iatt=1,numatt !Generate list |
|
if (abs(cubmat(attgrid(1,iatt),attgrid(2,iatt),attgrid(3,iatt)))>=threslowvalatt) then |
|
icounthigh=icounthigh+1 |
|
highvalatt(icounthigh)=iatt |
|
atttypelist(iatt)=1 !high |
|
end if |
|
end do |
|
nlowvalatt=numatt-icounthigh |
|
if (imode==2) then |
|
do iz=izlow,izup |
|
do iy=iylow,iyup |
|
do ix=ixlow,ixup |
|
if (atttypelist(gridbas(ix,iy,iz))==0) gridbas(ix,iy,iz)=0 |
|
end do |
|
end do |
|
end do |
|
else if (imode==3) then |
|
do iz=izlow,izup |
|
do iy=iylow,iyup |
|
do ix=ixlow,ixup |
|
call getgridxyz(ix,iy,iz,rnowx,rnowy,rnowz) |
|
if (atttypelist(gridbas(ix,iy,iz))==0) then !Find out which grid belongs to insignificant attractors |
|
shortmaxsqr=1D20 |
The orbital-composition export has the same class of defect: it assigns irealatt=gridbas(...) and uses that raw value to index 1-based composition arrays.
|
write(*,"(a)") " -4: Export composition of every basin in every orbital to orbcomp.txt" |
|
!write(*,"(a)") " -5: Print orbital delocalization index (ODI) for a batch of orbitals" |
|
read(*,"(a)") c80tmp |
|
|
|
if (c80tmp=="0") then |
|
if (allocated(frag1)) deallocate(frag1) |
|
return |
|
else if (c80tmp=="-1") then |
|
call showorbinfo(1,nmo) |
|
|
|
else if (c80tmp=="-4") then |
|
if (ifuncbasin/=1) then |
|
write(*,"(a)") " Error: This option is meaningful only when the function used to partition the basins is electron density!" |
|
write(*,*) "Press ENTER button to continue" |
|
read(*,*) |
|
cycle |
|
end if |
|
comparr=0D0 |
|
ifinish=0 |
|
!$OMP PARALLEL private(ix,iy,iz,irealatt,rnowx,rnowy,rnowz,comparrpriv,orbval) shared(ifinish,comparr) NUM_THREADS(nthreads) |
|
comparrpriv=0D0 |
|
!$OMP do schedule(DYNAMIC) |
|
do iz=izlow,izup |
|
do iy=iylow,iyup |
|
do ix=ixlow,ixup |
|
call getgridxyz(ix,iy,iz,rnowx,rnowy,rnowz) |
|
call orbderv(1,1,nmo,rnowx,rnowy,rnowz,orbval(:)) |
|
irealatt=gridbas(ix,iy,iz) |
|
comparrpriv(:,irealatt)=comparrpriv(:,irealatt)+orbval(:)**2 |
|
end do |
Impact
Any grid containing unassigned, boundary, or other non-positive sentinel cells can trigger out-of-bounds reads/writes when these menu paths are used. With bounds checking this can terminate immediately; optimized builds may silently corrupt memory or attribute values to unrelated basins.
Suggested direction
- Treat
gridbas <= 0 as an explicit non-attractor case before every attractor-array lookup.
- Preserve or deliberately remap each sentinel instead of passing it through 1-based arrays.
- Centralize basin-ID validation/conversion in a helper so analysis paths cannot repeat raw indexing.
- Add bounds-checked regressions containing
-1 and 0 cells for low-value elimination modes 2/3 and orbital-composition export.
Global scan provenance
This issue was found by the Codex global repository scan of non-unit-test files in
Stardust0831/Multiwfn, pinned to commitd2b770a531eb7039153198d04520b5992f15c49d.Problem
Basin generation deliberately stores non-attractor sentinel IDs in
gridbas:0for unassigned grids and-1for trajectories reaching an isolated-system box boundary.Multiwfn/basin.f90
Lines 745 to 753 in d2b770a
Multiwfn/basin.f90
Lines 1072 to 1077 in d2b770a
Low-value-attractor elimination then uses the raw
gridbasvalue to indexatttypelist, whose declared lower bound is 1:Multiwfn/basin.f90
Lines 1200 to 1237 in d2b770a
The orbital-composition export has the same class of defect: it assigns
irealatt=gridbas(...)and uses that raw value to index 1-based composition arrays.Multiwfn/basin.f90
Lines 2162 to 2191 in d2b770a
Impact
Any grid containing unassigned, boundary, or other non-positive sentinel cells can trigger out-of-bounds reads/writes when these menu paths are used. With bounds checking this can terminate immediately; optimized builds may silently corrupt memory or attribute values to unrelated basins.
Suggested direction
gridbas <= 0as an explicit non-attractor case before every attractor-array lookup.-1and0cells for low-value elimination modes 2/3 and orbital-composition export.