Opened 4 years ago

Closed 4 years ago

#6399 closed defect (fixed)

VICARKeywordHandler::Ingest() broken for End Of file Labels after 2.0.2 update

Reported by: swalter75 Owned by: warmerdam
Priority: normal Milestone: 2.0.3
Component: default Version: 2.0.2
Severity: normal Keywords: vicar
Cc:

Description (last modified by swalter75)

There were potential buffer overflows in pre-2.0 versions of gdal, which have been corrected in [32027] and later changesets. The correction was to truncate the amount of bytes read to that of the buffer. This resulted in non-functional (but secure) code.

Here we introduce another buffer variable with the correct size to read the full EOL label.

There are also other minor corrections in the patch, as well as a new label entry to let the user know that the driver is handling the #5941 bug correctly.

Attachments (2)

vicarfmt-6399.patch (6.2 KB) - added by swalter75 4 years ago.
vicarfmt-6399a.patch (6.1 KB) - added by swalter75 4 years ago.
Changed patch according to comment #2

Download all attachments as: .zip

Change History (8)

Changed 4 years ago by swalter75

Attachment: vicarfmt-6399.patch added

comment:1 Changed 4 years ago by swalter75

Description: modified (diff)

comment:2 Changed 4 years ago by Even Rouault

pszEOLHeader is not freed. and the return value of the VSIMalloc for pszChunkEOL is not tested against NULL

Changed 4 years ago by swalter75

Attachment: vicarfmt-6399a.patch added

Changed patch according to comment #2

comment:3 Changed 4 years ago by Even Rouault

I've a bit modified your patch to incorporate trunk changes, and also merged it into trunk. As this wasn't completely trivial, I'd appreciate if you could test that both trunk and 2.0 branches are OK. I've also added a "EOLabelSize > 100 * 1024 * 1024" test to bail out, to avoid potential issues with corrupted/hostile datasets.

trunk r33668, branches/2.0 r33669 "VICAR: fix regression regarding end-of-file labels (patch by Sebastian Walter, #6399)"

comment:4 Changed 4 years ago by swalter75

I've tested both trunk and 2.0 branch against my full data repository of vicar files - there seem to be no problems. Does it make sense to change the dirver sources also for the gdal 1.11.x branch? Sebastian

comment:5 in reply to:  4 Changed 4 years ago by Even Rouault

Replying to swalter75:

Does it make sense to change the dirver sources also for the gdal 1.11.x branch?

The VICAR driver was new to GDAL 2.0 ;-)

comment:6 Changed 4 years ago by Even Rouault

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.