Opened 13 years ago

Closed 13 years ago

Last modified 8 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 13 years ago.
Made the GCP list dynamically sized

Download all attachments as: .zip

Change History (7)

comment:1 Changed 13 years ago by warmerdam

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 Changed 13 years ago by valisystem

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 Changed 13 years ago by chaitanya

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 Changed 13 years ago by warmerdam

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.

Changed 13 years ago by chaitanya

Attachment: bsbdataset.patch added

Made the GCP list dynamically sized

comment:5 Changed 13 years ago by Even Rouault

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 Changed 8 years ago by Even Rouault

Milestone: 1.5.5

Milestone 1.5.5 deleted

Note: See TracTickets for help on using tickets.