Opened 14 years ago

Closed 14 years ago

Last modified 9 years ago

#2777 closed defect (fixed)

Problem with gcps count in BSB Driver

Reported by: valisystem Owned by: chaitanya
Priority: normal Milestone:
Component: GDAL_Raster Version: unspecified
Severity: normal Keywords: BSB
Cc: warmerdam, chaitanya

Description

the function BSBDataset::ScanForGCPs() sets a limit (MAX_GCP) for gcps number for allocation, but the implementation reads data and sets nGCPCount beyond that limit, so that GDALDataset->GetGCPs() returns corrupted (overwritten) data over the 256 limit.

See the map 14500_1 in the noaa RNC BSB nautical charts as an example.

Attachments (1)

bsbdataset.patch (2.3 KB ) - added by chaitanya 14 years ago.
Made the GCP list dynamically sized

Download all attachments as: .zip

Change History (7)

comment:1 by warmerdam, 14 years ago

Cc: warmerdam added
Component: defaultGDAL_Raster
Keywords: BSB added; corrupted data bsb kap gcps removed
Owner: changed from warmerdam to chaitanya

Hi,

Could you provide a direct pointer (url) to an example BSB demonstrating this problem?

Chaitanya, please reproduce the problem once a dataset is available, and prepare a proposed patch.

comment:2 by valisystem, 14 years ago

14500_1.KAP (use save link as)

Note that i discovered this bug while using the GDAL API, but a symptom is segfault/malloc warning with gdalinfo on the file.

Another file with more than 256 gcps is 16568_1.KAP (from the noaa rncs).

comment:3 by chaitanya, 14 years ago

Cc: chaitanya added

Frank, The functions

BSBDataset::ScanForGCPsBSB()

and

BSBDataset::ScanForGCPsNos( const char *pszFilename )

are ignoring the

#define MAX_GCP 256

value. They should compare the

nGCPCount

value with MAX_GCP in their for-loop and while-loop respectively and stop scanning further. Otherwise, I can write some code to make the nGCPCount flexible?

comment:4 by warmerdam, 14 years ago

Chaitanya,

Please prepare a patch making the GCP list dynamically sized. Try to properly use GDALInitGCPs() to intialize extended portions of the gcp array so that future changes in the structure will be ok even if we don't revisit the BSB driver.

by chaitanya, 14 years ago

Attachment: bsbdataset.patch added

Made the GCP list dynamically sized

comment:5 by Even Rouault, 14 years ago

Milestone: 1.5.5
Resolution: fixed
Status: newclosed

Chaitanya,

I've applied your patch in trunk in r16166, in branches/1.6 in r16167 and in branches/1.5 in r16168.

While reviewing surrounding code, I've also found some security or memory usage problems that I've also fixed in those commits.

comment:6 by Even Rouault, 9 years ago

Milestone: 1.5.5

Milestone 1.5.5 deleted

Note: See TracTickets for help on using tickets.