Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenFileGDB: be robust to unusual .gdbindexes files with weird/corrupted/not-understood entries #11372

Merged
merged 1 commit into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿ
16 changes: 16 additions & 0 deletions autotest/ogr/ogr_openfilegdb.py
Original file line number Diff line number Diff line change
Expand Up @@ -2942,3 +2942,19 @@ def test_ogr_openfilegdb_arc_interior_point_bug_line():
f,
"MULTILINESTRING ((37252520.1717 7431529.9154,38549084.9654 758964.7573))",
)


###############################################################################
# Test https://github.com/OSGeo/gdal/issues/11295 where .gdbindexes contains
# entry with unusual (corrupted ? disabled?) entries


def test_ogr_openfilegdb_weird_gdbindexes():

# File a00000009.gdbindexes has been replaced by file a00000029.gdbindexes
# from attachment of https://github.com/OSGeo/gdal/issues/11295#issuecomment-2491158506
with ogr.Open("data/filegdb/corrupted_gdbindexes.gdb") as ds:
lyr = ds.GetLayer(0)
lyr.SetAttributeFilter("id = '1'")
f = lyr.GetNextFeature()
assert f
55 changes: 51 additions & 4 deletions ogr/ogrsf_frmts/openfilegdb/filegdbtable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2537,6 +2537,7 @@ int FileGDBTable::GetIndexCount()
CPLDebug("OpenFileGDB", ".gdbindexes v9 not handled yet");
return 0;
}

returnErrorAndCleanupIf(nIndexCount >=
static_cast<size_t>(GetFieldCount() + 1) * 10,
VSIFree(pabyIdx));
Expand All @@ -2547,7 +2548,7 @@ int FileGDBTable::GetIndexCount()
returnErrorAndCleanupIf(static_cast<GUInt32>(pabyEnd - pabyCur) <
sizeof(GUInt32),
VSIFree(pabyIdx));
GUInt32 nIdxNameCarCount = GetUInt32(pabyCur, 0);
const GUInt32 nIdxNameCarCount = GetUInt32(pabyCur, 0);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was Car actually meant to be Char ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addressed in #11410

pabyCur += sizeof(GUInt32);
returnErrorAndCleanupIf(nIdxNameCarCount > 1024, VSIFree(pabyIdx));
returnErrorAndCleanupIf(static_cast<GUInt32>(pabyEnd - pabyCur) <
Expand All @@ -2557,13 +2558,56 @@ int FileGDBTable::GetIndexCount()
ReadUTF16String(pabyCur, nIdxNameCarCount));
pabyCur += 2 * nIdxNameCarCount;

// 4 "magic fields"
returnErrorAndCleanupIf(static_cast<GUInt32>(pabyEnd - pabyCur) <
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not follow: if the comparison is between a sum of size_t (that I assume to be size_t itself) why cast to GUInt32 ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addressed in #11410

sizeof(GUInt16) + sizeof(GUInt32) +
sizeof(GUInt16) + sizeof(GUInt32),
VSIFree(pabyIdx));
// const GUInt16 nMagic1 = GetUInt16(pabyCur, 0);
const GUInt32 nMagic2 = GetUInt32(pabyCur + sizeof(GUInt16), 0);
const GUInt16 nMagic3 =
GetUInt16(pabyCur + sizeof(GUInt16) + sizeof(GUInt32), 0);
if (!((nMagic2 == 2 && nMagic3 == 0) ||
(nMagic2 == 4 && nMagic3 == 0) ||
(nMagic2 == 16 && nMagic3 == 65535)))
{
// Cf files a00000029.gdbindexes, a000000ea.gdbindexes, a000000ed.gdbindexes,
// a000000f8.gdbindexes, a000000fb.gdbindexes, a00000103.gdbindexes
// from https://github.com/OSGeo/gdal/issues/11295#issuecomment-2491158506
CPLDebug("OpenFileGDB", "Reading %s", pszIndexesName);
CPLDebug(
"OpenFileGDB",
"Strange (deleted?) index descriptor at index %u of name %s", i,
osIndexName.c_str());

// Skip magic fields
pabyCur += sizeof(GUInt16);

const GUInt32 nColNameCarCount = nMagic2;
pabyCur += sizeof(GUInt32);
returnErrorAndCleanupIf(nColNameCarCount > 1024, VSIFree(pabyIdx));
returnErrorAndCleanupIf(static_cast<GUInt32>(pabyEnd - pabyCur) <
2 * nColNameCarCount,
VSIFree(pabyIdx));
pabyCur += 2 * nColNameCarCount;

// Skip magic field
returnErrorAndCleanupIf(static_cast<GUInt32>(pabyEnd - pabyCur) <
sizeof(GUInt16),
VSIFree(pabyIdx));
pabyCur += sizeof(GUInt16);

continue;
}

// Skip magic fields
pabyCur += 2 + 4 + 2 + 4;
pabyCur += sizeof(GUInt16) + sizeof(GUInt32) + sizeof(GUInt16) +
sizeof(GUInt32);

returnErrorAndCleanupIf(static_cast<GUInt32>(pabyEnd - pabyCur) <
sizeof(GUInt32),
VSIFree(pabyIdx));
GUInt32 nColNameCarCount = GetUInt32(pabyCur, 0);
const GUInt32 nColNameCarCount = GetUInt32(pabyCur, 0);
pabyCur += sizeof(GUInt32);
returnErrorAndCleanupIf(nColNameCarCount > 1024, VSIFree(pabyIdx));
returnErrorAndCleanupIf(static_cast<GUInt32>(pabyEnd - pabyCur) <
Expand All @@ -2574,7 +2618,10 @@ int FileGDBTable::GetIndexCount()
pabyCur += 2 * nColNameCarCount;

// Skip magic field
pabyCur += 2;
returnErrorAndCleanupIf(static_cast<GUInt32>(pabyEnd - pabyCur) <
sizeof(GUInt16),
VSIFree(pabyIdx));
pabyCur += sizeof(GUInt16);

auto poIndex = std::make_unique<FileGDBIndex>();
poIndex->m_osIndexName = osIndexName;
Expand Down
Loading