Skip to content

Envisat: fix heap overflow in ScanForGCPs_ASAR from unchecked NUM_DSR#14941

Merged
rouault merged 1 commit into
OSGeo:masterfrom
saddamr3e:envisat-gcp-num-dsr-overflow-pr
Jul 16, 2026
Merged

Envisat: fix heap overflow in ScanForGCPs_ASAR from unchecked NUM_DSR#14941
rouault merged 1 commit into
OSGeo:masterfrom
saddamr3e:envisat-gcp-num-dsr-overflow-pr

Conversation

@saddamr3e

Copy link
Copy Markdown
Contributor

What does this PR do?

EnvisatDataset::ScanForGCPs_ASAR() sizes the GCP array from the NUM_DSR field of the GEOLOCATION GRID ADS descriptor:

pasGCPList = (GDAL_GCP *)CPLCalloc(sizeof(GDAL_GCP), (nNumDSR + 1) * 11);

nNumDSR is taken verbatim from the dataset descriptor and (nNumDSR + 1) * 11 is evaluated as int. A large declared count overflows: (390451572 + 1) * 11 wraps to 7, so the array is under-allocated while the per-record loop keeps writing 11 GCPs per record through GDALInitGCPs() past the end of the buffer.

It triggers on a small crafted .N1 with a single 521-byte geolocation record and NUM_DSR=390451572. With -ftrapv (GDAL Debug builds) the multiply traps; without it AddressSanitizer reports:

ERROR: AddressSanitizer: heap-buffer-overflow ... WRITE of size 56
    #1 GDALInitGCPs gdal_misc.cpp:1884
    #2 EnvisatDataset::ScanForGCPs_ASAR() envisatdataset.cpp:335
    #3 EnvisatDataset::Open(GDALOpenInfo*) envisatdataset.cpp:1140
0x... is located 0 bytes after 392-byte region
allocated by ... EnvisatDataset::ScanForGCPs_ASAR() envisatdataset.cpp:313

The fix stays in the callee: reject a NUM_DSR that cannot fit the declared dataset size, and compute the array size in size_t, the same way ScanForGCPs_MERIS() already does. Valid products read identically; the added test builds a well-formed geolocation grid that still yields 33 GCPs.

What are related issues/pull requests?

None. Found by review of the GCP collection paths.

AI tool usage

  • AI (Y-a-t-il-un-Copilot-dans-l'avion, Chat-j'ai-pété, Jean-Claude Dusse or something similar) supported my development of this PR. See our policy about AI tool use. Use of AI tools must be indicated.

Tasklist

  • Make sure code is correctly formatted (cf pre-commit configuration)
  • Add test case(s)
  • Add documentation
  • Updated Python API documentation (swig/include/python/docs/)
  • Review
  • Adjust for comments
  • All CI builds and checks have passed

Environment

  • OS: any
  • Compiler: any

NUM_DSR from the GEOLOCATION GRID ADS sizes the GCP array via (nNumDSR + 1) * 11 evaluated as int, which overflows for a large declared count and under-allocates the array while the loop keeps writing 11 GCPs per record past its end. Reject a NUM_DSR that cannot fit the declared dataset size and compute the size in size_t, as ScanForGCPs_MERIS already does.
@coveralls

Copy link
Copy Markdown
Collaborator

Coverage Status

coverage: 73.764% (+0.06%) from 73.706% — saddamr3e:envisat-gcp-num-dsr-overflow-pr into OSGeo:master

@rouault rouault added the backport release/3.13 Backport to release/3.13 label Jul 16, 2026
@rouault
rouault merged commit cf656de into OSGeo:master Jul 16, 2026
33 of 37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport release/3.13 Backport to release/3.13

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants