osgeo / shapelib Goto Github PK

Hi,

Our Cisco AMP has system has picked up the compiled version of shapelib.dll as containing a potential security threat.

The log is as follows:-

PE Has Sections Marked Executable and Writable
Score: 24 Hits: 2
Description
Usually, code sections are marked as Readable/Non-Writable/Executable, which tells the operating system that the corresponding memory locations contain executable code and write operations should be forbidden. On the other hand, data sections are usually marked as Readable/Writable/Non-Executable, and therefore the Program Counter should never point to memory locations in the range of data sections. The presence of both the IMAGE_SCN_MEM_EXECUTE and the IMAGE_SCN_MEM_WRITE Flags could indicate the executable is packed or contains some other type of self modifying code.

Trigger
A PE containing a section that is both writable and executable will trigger this indicator.

Is it possible to update the code to remove these triggers ?

Thanks

I'm taking a look at what it might look like to cleanup shapelib and move it to C99 or newer. I know that opinions vary as to if these are good changes or not, so I'm doing this in a separate repo without worrying out making PRs:

https://github.com/schwehr/shapelib-experimental

I used debian testing with -Wall -Wextra and cppcheck to keep an eye on issues.

The changes are not always fully implemented. The goal is evaluate what the library might look like. There is not currently enough testing in shapelib to be sure that nothing has broken. I'm hoping to find some time to add more test coverage. I did put the core files into my local copy of GDAL and my shape tests pass.

So far, the changes I've made are roughly:

• clang-format
• Remove unused variables
• Add missing include of stdio.h (beginnings of IWYU)
• Order includes
• Remove comments that are no longer relevant
• Remove cvs log messages from file comments
• Reduce bulky block comments
• Localize variables
• Combine definition and initialization
• Add const when possible
• int boolean → bool (using stdbool.h)
• Remove dbmalloc
• Remove C++ compilation of the C source (removed C++ casting macros)
• Mark a few possible bugs with TODO
• Reorder functions to not need local prototypes
• Fix some error exit's to properly close open resources

This has already reduced the number of lines of source files:

# Before
wc *.[ch] contrib/*.[ch] | tail -1
 15995  58264 553965 total

# Currently
wc *.[ch] contrib/*.[ch] | tail -1
 13566  48865 469071 total

There are a lot more things I'd like to try out including:

• Add C test coverage (maybe with catch2? or I could default to gtest)
• Add command line tests using python3 to exercise the resulting programs
• Fuzzing directly on shapelib (both to find bugs and to make it easy to find files for testing)
• Use stdint.h types
• Use C99 printf codes
• Remove DEBUG code
• convert int foo; if (a) foo=b; else foo=c; → const int foo = a ? b : c;
• Running tests in asan and msan modes
• Looking at coverity
• etc.

Some of the issues I've found so far:

• A variety of license issues. e.g. files that state public domain and contain a copyright
• I'm not sure if some of the contrib files work
• booleans that are true, false, and sometimes -1
• Missing conditions in if chains / switch statements. e.g. dbfdump.c
• Variables with incorrect Hungarian prefixes. e.g. bMinX for int and iunselect for bool
• Multiple endian check and a few other redundancies
• Unchecked malloc and realloc calls that may fail
• Variables that don't change causing code to be unreachable. e.g. byRing
• shpdxf.c has a huge number of warnings including may be used uninitialized
• CMake doesn't run the tests
• shputils.c has a large number of globals and is very hard to follow
• man pages would be nice
• functions should have const on pointer args they do not alter

We are using shptest as a testsuite for the Julia package JuliaGeo/Shapefile.jl#40. We are confused by the fact that the test polygons seem to run in the "wrong" direction.

The polygons defined here

So, isn't the generated shapefile invalid, or am I getting something wrong here?

Follow up to #17 , I am going to split out tasks starting with easier / less controversial (I hope)

The first one is that there are quite a few unused variables, especially in contrib. Most of these look like copy/paste remnants. Others are more tricky. e.g. defined in one place, set in one or more places, and never used.

Following up on #17, this repo doesn't have man pages. I'm contacting the debian folks (Bas Couwenberg and Francesco Paolo Lovergine) to see about using the ones they package as a starting point.

http://deb.debian.org/debian/pool/main/s/shapelib/shapelib_1.5.0-2.debian.tar.xz

From rules:

ronn -r --date="$(BUILD_DATE)" --manual=shplib debian/man/*.md

It has these markdown sources for man pages:

ls -1 man
dbfadd.md
dbfcat.md
dbfcreate.md
dbfdump.md
dbfinfo.md
Shape_PointInPoly.md
shpadd.md
shpcat.md
shpcentrd.md
shpcreate.md
shpdata.md
shpdump.md
shpdxf.md
shpfix.md
shpinfo.md
shprewind.md
shpsort.md
shptreedump.md
shputils.md
shpwkb.md

In dbfopen.c file we have a function DBFIsValueNULL(), in the comment you have mentioned
/*
** We accept all asterisks or all blanks as NULL
** though according to the spec I think it should be all
** asterisks.
/
/ NULL boolean fields have value "?" */

which specification tells that '*' is NULL for numeric and '?' for Boolean. Can you please explain and provide the appropriate link supporting the same. I am new to shapelib and github both , my apologies if i have asked the question in wrong forum please assist.

Compiling on a newer compiler yields the warning:

gcc-9  -I"/home/hornik/tmp/R/include" -DNDEBUG  -I'/home/hornik/lib/R/Library/4.1/x86_64-linux-gnu/Rcpp/include' -I/usr/local/include -DUSE_TYPE_CHECKING_STRICT -D_FORTIFY_SOURCE=2  -fpic  -g -O2 -Wall -pedantic -mtune=native  -c dbfopen.c -o dbfopen.o
In file included from /usr/include/string.h:495,
                 from dbfopen.c:192:
In function ‘strncpy’,
    inlined from ‘DBFWriteAttribute.part.0’ at dbfopen.c:1479:9:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: warning: ‘__builtin_strncpy’ output may be truncated copying between 0 and 255 bytes from a string of length 255 [-Wstringop-truncation]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The warning specifically points out that with strncpy it's possible that (pabyRec+psDBF->panFieldOffset[iField]) will not end up null-terminated resulting in buffer vulnerabilities; leastwise, the compiler cannot prove that it isn't null-terminated.

(This is problematic because it means I can't use shapelib in R's CRAN repositories.)

It was reported by a PostGIS user, who provided a patch for our copy of shapelib:
https://git.osgeo.org/gitea/postgis/postgis/pulls/40

Initially, we need some discussion on this topic.

Need to create a release process for shapelib.

• How to handle version numbering? That last release was 1.5.0. What's next?
• Where to set the version number? Can it be done from one location? It's currently in configure.ac and CMakeList.txt
• How to manage additions to the ChangeLog?
• github action to generate rc and/or final candidate tar files
• github action to generate binaries for primary platforms

There should be only on place that checks for endian and it should be at compile time.

What do autoconf and CMake say about endianness?

• AC_C_BIGENDIAN
• TEST_BIG_ENDIAN

https://stackoverflow.com/questions/2100331/c-macro-definition-to-determine-big-endian-or-little-endian-machine

http://esr.ibiblio.org/?p=5095

With most cmake projects I can include them in another cmake project like so:

add_subdirectory(submodule/shapelib)
[...]
target_link_libraries(my_target PRIVATE shp)

With shapelib this doesn't work:

../src/my_code:30:10: fatal error: shapefil.h: No such file or directory
   30 | #include <shapefil.h>

I haven't been able to figure out why this is yet, but it looks as though the shp target is not getting its include directory set.

The library is currently licensed under GPL which makes it impossible to use in a wide range of applications. Would you consider re-licensing it under a less demanding license such as MIT?
Thank you!

Summary

The buffer copy is freed twice, leading to possible memory corruption or vulnerability.

Cause

In split(), at shpsort.c:107, the buffer copy is free'd. realloc() fails on line 110, the buffer copy is freed again at shpsort.c:116.

Impact

A double-free bug can lead to an attacker gaining control over the values returned from malloc(), which in turn may allow both disclosure of sensitive data (e.g. bypassing additional safety features) or in the worst case hostile code execution.

Vulnerable version

• contrib/shpsort (commit 21ae8fc)

Proposed mitigation

Remove line 116, as it is redundant.

README.CMake mentions an optional shapelib.patch file for error message silencing. It is not obvious where to find this file. (I failed.)

1. Should this step in the Readme file simply be removed?
2. If not, where can it be found?
3. Should error message silencing be made a configuration flag?

The CMake install target installs the header file twice:

-- Installing: C:/install_dir/include/shapefil.h
-- Installing: C:/install_dir/include/shapelib/shapefil.h

Is this by design? If not, which is the expected location?

however the configure flow does create shapelib.pc.

The code in shputils.c was never finished and is pretty messy. There are no tests for it. Someone needs to adopt this code. I've done a bunch of cleanup on it, but it's difficult to work with.

SHPLIB_NULLPTR is defined in a number of places. e.g. shpopen.c

#ifdef __cplusplus
#define STATIC_CAST(type,x) static_cast<type>(x)
#define SHPLIB_NULLPTR nullptr
#else
#define STATIC_CAST(type,x) ((type)(x))
#define SHPLIB_NULLPTR NULL
#endif

This doesn't buy much as there is not mechanism in shapelib to build as C++. If the internals are ever converted to C++, then most of the NULLs should be converted to nullptr. The idea of having it both ways just adds complexity. Currently:

grep SHPLIB_NULLPTR *.c | wc -l
245

grep NULL *.c | grep -v SHPLIB_NULLPTR | wc -l
92

@rouault added the cast macros and SHPLIB_NULLPTR in 8ba2778. However, looking at gdal/ogr/ogrsf_frmts/shape, it doesn't currently look like shapelib is being built as C++ in sGDAL

Now that gdal 3.8.5 is released and bundles the latest shapelib fixes, it would be appreciated to also have a new stand-alone release of shapelib containing theses fixes. Thanks.

By the way, this kind of dependency management seems odd actually. What about using shapelib as submodule within gdal? Or pulling shaplib otherwise else?

I think that it would be good to flush currently committed changes and make new release :)

Thanks to @dbaston for pointing out that there are places other than gdal that shapelib is included.

And specifically:

• Which parts of shapelib?
• What is the best link for each?
• Do they add anything?

Packages

• gdal/ogr/ogrsf_frmts/shape
• postgis/loader pgsql2shp and shp2pgsql

Package maintainers (only include if they directly package part of shapelib).

• Debian/Ubuntu - Adds man pages
• Fedora
• FreshPorts - FreeBSD
• CondaForge
• NetBSD pkgsrc

I am going to hold off on anything that is a part of the public API, but internally there are a good number of cases where the int type can be converted to stdbool. This means that TRUE & FALSE can be converted to the standard true & false.

It's important to watch out for cases where a variable can also have a value other that
TRUE / FALSE. Often that is -1.

• https://en.cppreference.com/w/c/types/boolean
• https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/stdbool.h.html
• https://www.cplusplus.com/reference/cstdbool/

grep 'int b' *.c */*.c | grep -v byRing | wc -l
29

e.g.

dbfopen.c:            const int bIsNULL = DBFIsValueNULL( chOldType, pszOldField );
sbnsearch.c:    int bBigEndian;
sbnsearch.c:    int bMinX;
shpopen.c:    const int bFirstFeature = psSHP->nRecords == 0;

realloc is supposed to be equivalent to

static void * SfRealloc( void * pMem, int nNewSize )

{
    if( pMem == NULL )
        return( malloc(nNewSize) );
    else
        return( realloc(pMem,nNewSize) );
}

https://en.cppreference.com/w/c/memory/realloc

If ptr is NULL, the behavior is the same as calling malloc(new_size).

Hello,
wanted to ask, why integer fields with width 10 are saved as double field.
This logic was changed in DBFGetFieldInfo few years ago and IMHO it's not what it should do.
Code:

 if( psDBF->panFieldDecimals[iField] > 0
            || **psDBF->panFieldSize[iField] >= 10** )
	    return( FTDouble );
	else
	    return( FTInteger );

Thanks...

I'm using shapelib on OSX using vcpkg, and it doesn't play very well with everything else which just assumes that installed libraries will be found under @rpath--for example:

$ otool -L /Users/russellgreene/ars/ext/vcpkg/packages/freetype_arm64-osx-dynamic/lib/libfreetype.6.18.3.dylib
/Users/russellgreene/ars/ext/vcpkg/packages/freetype_arm64-osx-dynamic/lib/libfreetype.6.18.3.dylib:
        @rpath/libfreetype.6.dylib (compatibility version 6.0.0, current version 6.18.3)
        @rpath/libz.1.dylib (compatibility version 1.0.0, current version 1.2.13)
        @rpath/libbz2.1.0.dylib (compatibility version 1.0.0, current version 1.0.6)
        @rpath/libpng16.16.dylib (compatibility version 16.0.0, current version 16.39.0)
        @rpath/libbrotlidec.1.dylib (compatibility version 1.0.0, current version 1.0.9)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1319.0.0)

however, shp looks like

$ otool -L /Users/russellgreene/ars/ext/vcpkg/packages/shapelib_arm64-osx-dynamic/debug/lib/libshp.1.dylib
/Users/russellgreene/ars/ext/vcpkg/packages/shapelib_arm64-osx-dynamic/debug/lib/libshp.1.dylib:
        lib/libshp.1.dylib (compatibility version 1.0.0, current version 1.5.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1319.0.0)

removing the INSTALL_NAME_DIR directive fixes this and brings it towards "normal" behavior.

Is there a good reason for this change? If not, would a PR removing it be accepted? Thank you!

Shapelib the library already compiles pretty cleanly as C++. What do people think about making that always the case. I propose keeping the API as C, but we can then have things like constexpr and a cleanup class that would make the code easier to follow. Things I'm thinking about:

• constexpr can be used for the length of arrays
• using unique_ptr for internal only allocations
• Something like absl::Cleanup with lambda's greatly simplify returns from functions that are holding resources (like open hdf or shp file references)
• and...

#25 talks about these:

#ifdef __cplusplus
#define STATIC_CAST(type,x) static_cast<type>(x)
#define REINTERPRET_CAST(type,x) reinterpret_cast<type>(x)
#define CONST_CAST(type,x) const_cast<type>(x)
#define SHPLIB_NULLPTR nullptr
#else
#define STATIC_CAST(type,x) ((type)(x))
#define REINTERPRET_CAST(type,x) ((type)(x))
#define CONST_CAST(type,x) ((type)(x))
#define SHPLIB_NULLPTR NULL
#endif

Thoughts?

Compiling shapelib using (I think) any modern cmake (2.8.12+) gives a number of warnings:

CMake Warning (dev) at submodule/shapelib/CMakeLists.txt:179 (get_target_property):
  Policy CMP0026 is not set: Disallow use of the LOCATION target property.
  Run "cmake --help-policy CMP0026" for policy details.  Use the cmake_policy
  command to set the policy and suppress this warning.

  The LOCATION property should not be read from target "dbfdump".  Use the
  target name directly with add_custom_command, or use the generator
  expression $<TARGET_FILE>, as appropriate.

If the SHX file is missing the following error message is logged.

I wonder what the SHAPE_RESTORE_SHX config option is.

Hello, has anyone had problems with the use of the pascal version of the file 'ShapefileII.pas'?
I'm trying to create a shp file and the associated .DBF file using Delphi. I can create the SHP and DBF file, but I cannot add string data to the DBF file. I used the steps described in the example file 'dbfadd.c' using the function 'DBFWriteStringAttribute' and 'DBFWriteIntegerAttribute'. The function does not return any errors, but the string field in DBF file is not updated.
Please, can anyone help me?
Here my code:

//add data
FDBFHandle := DBFOpen(pAnsichar(FileNameDBF), pAnsichar('r+b')); //open a DBF file created
if FDBFHandle = nil then
exit;
iRecord:= DBFGetRecordCount(FDBFHandle);
DBFWriteIntegerAttribute(FDBFHandle, iRecord, 0, 99 ); //works fine
DBFWriteStringAttribute (FDBFHandle, iRecord, 1, PAnsichar('TEST' ));//NOT WORK BUT RETURN TRUE
//Using PAnsistring also not work
DBFClose(FDBFHandle);

Currently the implementation of the field FTDate is a bit basic:

• DBFReadDateAttribute is missing. Currently the best way is to use DBF2ReadStringAttribute (like QDate::fromString(DBF2ReadStringAttribute(dbfHandle, shapeId, field), "yyyyMMdd")) as e.g. DBF2ReadIntegerAttribute makes it more complex to extract the year, month and date. An API like SHP_DATE SHPAPI_CALL1(*) DBFReadStringAttribute( DBFHandle psDBF, int iRecord, int iField ) would be nice.
• DBFWriteDateAttribute is missing. You need to know that you need to use DBFWriteIntegerAttribute or DBFWriteDoubleAttribute. Furthermore you need to know the format to use write it to db file (our call e.g. attribute.getDate().toString("yyyyMMdd").toInt()). A API like DBFWriteDateAttribute( DBF2Handle psDBF2, int iRecord, int iField, int year, int month, int day) would be nice, so you don't need to know the date format (or a struct instead of the 3 values)
• As you can only use numeric API to write date, it is not possible to write a proper invalid value. The date should be in format YYYYMMDD. E.g. in QGIS an invalid value is written as 00000000, during this is not possible in this lib,. as it is handled as integer you can only write 0

shapelib should have github actions that at least test on one platform.

Upon compilation I get the following warnings, which seem potentially serious since it looks as though FTDate will otherwise get skipped. FTDate is returned by DBFGetFieldInfo.

shapelib/shputils.c:396:25: warning: enumeration value 'FTDate' not handled in switch [-Wswitch]
                switch( DBFGetFieldInfo( hDBF, i, NULL, &iWidth, &iDecimals ) )
                        ^
shapelib/shputils.c:675:17: warning: enumeration value 'FTDate' not handled in switch [-Wswitch]
        switch( DBFGetFieldInfo( hDBF, i, iszTitle, &iWidth, &iDecimals ) )
                ^
shapelib/shputils.c:905:1: warning: control may reach end of non-void function [-Wreturn-type]
}

The current testing setup only runs command line programs and compares their results to golden files. shapelib needs unit tests that direct call the C functions with a much more diverse range of inputs. e.g. exercising various error conditions. There are lots of frameworks that would work well for this.

I'm thinking of going with Catch2. It's got a (slower) option to use a single header and source file to get started. And I've wanted to give it a try. Having these tests in C++ (probably >= C++17) will mean that some platforms will only be able to use the original shell script based testing, but that should be okay as these tests will exercise the code on at least the 4 configurations currently setup for CI in the project.

See the C and C++ sections in Wikipedia's C and C++ Frameworks in List of unit testing frameworks. Probably anything reasonably maintained and open source would be fine. Some of the options are:

Example starter based test in dbfopen_test.cc. Apologies for code that isn't totally clean and SetContents isn't particularly good.

catch2:

#include <filesystem>
#include <fstream>
#include <iostream>
#include <string>
#include <string_view>

#include "catch.hpp"
#include "shapefil.h"

namespace {

constexpr char kTestData[] = "testdata/";

bool SetContents(std::string_view file_name, std::string_view content) {
  std::ofstream file(file_name);
  if (!file.is_open()) return false;
  file << content;
  file.close();
  return true;
}

TEST_CASE("DBFOpen", "[dbfopen]") {
  SECTION("Open does not exist - rb") {
    auto handle = DBFOpen("/does/not/exist.dbf", "rb");
    REQUIRE(handle == nullptr);
  }

  SECTION("Open does not exist - rb+") {
    auto handle = DBFOpen("/does/not/exist2.dbf", "rb+");
    REQUIRE(handle == nullptr);
  }

  SECTION("Open not a dbf") {
    const std::string filename = kTestData + std::string("not_a_dbf.dbf");
    auto handle = DBFOpen(filename.c_str(), "rb");
    REQUIRE(handle == nullptr);
  }

  SECTION("Open and close a.dbf") {
    const std::string filename = kTestData + std::string("a.dbf");
    auto handle = DBFOpen(filename.c_str(), "rb");
    REQUIRE(handle != nullptr);
    DBFClose(handle);
  }
}

TEST_CASE("DBFCreate", "[dbfcreate]") {
  SECTION("DoesNotExist") {
    auto handle = DBFCreate("/does/not/exist");
    REQUIRE(nullptr == handle);
  }

  SECTION("CreateAlreadyExists") {
    const std::string filename = kTestData + std::string("in-the-way.dbf");
    REQUIRE(SetContents(filename, "some content"));
    auto handle = DBFCreate(filename.c_str());
    // TODO(schwehr): Seems like a bug to overwrite an existing.
    REQUIRE(nullptr != handle);
    DBFClose(handle);
    auto size = std::filesystem::file_size(filename);
    REQUIRE(34 == size);
  }

  SECTION("Create and close") {
    const std::string filename = kTestData + std::string("empty.dbf");
    auto handle = DBFCreate(filename.c_str());
    DBFClose(handle);
    auto size = std::filesystem::file_size(filename);
    REQUIRE(34 == size);
  }
}

}  // namespace

Almost the same thing written with GoogleTest:

#include <filesystem>
#include <fstream>
#include <iostream>
#include <string>
#include <string_view>

#include "gunit.h"
#include "shapefil.h"

namespace {

constexpr char kTestData[] = "testdata/";

bool SetContents(std::string_view file_name, std::string_view content) {
  std::ofstream file(file_name);
  if (!file.is_open()) return false;
  file << content;
  file.close();
  return true;
}

TEST(DbfOpenTest, testDoesNotExist) {
  auto handle = DBFOpen("/does/not/exist", "rb");
  EXPECT_EQ(nullptr, handle);
}

TEST(DbfOpenTest, testOpenNotDbf) {
  const std::string filename = kTestData + std::string("not_a_dbf.dbf");
  auto handle = DBFOpen(filename.c_str(), "rb");
  EXPECT_EQ(nullptr, handle);
}

TEST(DbfOpenTest, testOpenClose) {
  const std::string filename = kTestData + std::string(""a.dbf");
  auto handle = DBFOpen(filename.c_str(), "rb");
  EXPECT_NE(nullptr, handle);
  DBFClose(handle);
}

TEST(DbfCreateTest, testDoesNotExist) {
  auto handle = DBFCreate("/does/not/exist");
  EXPECT_EQ(nullptr, handle);
}

TEST(DbfCreateTest, testCreateAlreadyExists) {
  const std::string filename = kTestData + std::string("in-the-way.dbf");
  ASSERT_TRUE(SetContents(filename, "some content"));
  auto handle = DBFCreate(filename.c_str());
  // TODO(schwehr): Seems like a bug to overwrite an existing.
  EXPECT_NE(nullptr, handle);
  DBFClose(handle);
  auto size = std::filesystem::file_size(filename);
  EXPECT_EQ(34, size);
}

TEST(DbfCreateTest, testCreateClose) {
  const std::string filename = kTestData + std::string("empty.dbf");
  auto handle = DBFCreate(filename.c_str());
  DBFClose(handle);
  auto size = std::filesystem::file_size(filename);
  EXPECT_EQ(34, size);
}

}  // namespace

Here's a dataset (Shapefile with spatial index .sbn / .sbx)

• info
• data (zip)

for which ogrinfo returns:

ogrinfo -ro -so -spat 5 5 6 6 glwd_2.shp --debug on
Shape: DBF Codepage = LDID/87 for glwd_2.shp
Shape: Treating as encoding 'ISO-8859-1'.
GDAL: GDALOpen(glwd_2.shp, this=0x55765f5e0090) succeeds as ESRI Shapefile.
INFO: Open of `glwd_2.shp'
      using driver `ESRI Shapefile' successful.
OGR: GetLayerCount() = 1


Layer name: glwd_2
Metadata:
  DBF_DATE_LAST_UPDATE=2003-05-20
Geometry: Polygon
ERROR 1: Inconsistent shape count for bin
SHAPE: Used spatial index, got 0 matches.
Feature Count: 13
Extent: (-180.000000, -55.587208) - (180.000000, 83.575951)
Layer SRS WKT:
(unknown)
GLWD_ID: Integer64 (10.0)
TYPE: String (12.0)
POLY_SRC: String (12.0)
AREA_SKM: Real (12.1)
PERIM_KM: Real (12.1)
LONG_DEG: Real (10.2)
LAT_DEG: Real (10.2)
GDAL: GDALClose(glwd_2.shp, this=0x55765f5e0090)

What makes me wonder, is that this dataset originated from and is widely used in scientific context, and AFAICS nobody complained about a corrupt spatial index so far.

So could it be, that the index is correct and the shown error is a false positive?

Looks like the error comes from here: