Opened 13 years ago

Closed 13 years ago

#1563 closed defect (fixed)

Input handling error in gsagdataset.cpp

Reported by: kevinoid Owned by: dron
Priority: normal Milestone: 1.5.0
Component: GDAL_Raster Version: unspecified
Severity: normal Keywords:
Cc: warmerdam


If GSAGDataset::IReadBlock() gets unlucky and happens to fill szLineBuf so that it ends in either a '-' or '+', it will print a warning and discard the sign as it continues to read the block. Instead it should either keep or re-read the sign and continue without warning. I have included a patch that implements the second option.

In fixing this I also realized that when there is a '\0' in the file it will cause the reader to loop indefinitely. The attached patch fixes this issue as well.

This problem is present in all versions of gsagdataset.cpp up to revision 11219 (HEAD at the time of this writing).

Attachments (1)

gdal-gsagdataset_input_handling.patch (1.9 KB) - added by kevinoid 13 years ago.
Patch to fix input handling errors in GSAGDataset::IReadBlock

Download all attachments as: .zip

Change History (4)

comment:2 Changed 13 years ago by warmerdam

Cc: warmerdam added
Milestone: 1.5.0
Owner: changed from warmerdam to dron


I think you were taking care of this driver? If you want, you can reassign to Mateusz to take care off.

comment:3 Changed 13 years ago by kevinoid

I just noticed that there is another instance of this problem that is currently not appearing only because CPLStrtod() behaves differently than the glibc and BSD libc strtod() for inputs that end in E or E+ or E-. For example, when run on the string "1.0E" strtod() will set tailptr to "E" while CPLStrtod() will set it to the '\0' following E. Should this behavior ever change to match these implementations, GSAGDataset::IReadBlock will misread the exponents as separate numbers and skew the data around. As a precaution, I have updated the patch to protect against this.

Changed 13 years ago by kevinoid

Patch to fix input handling errors in GSAGDataset::IReadBlock

comment:4 Changed 13 years ago by dron

Resolution: fixed
Status: newclosed

I have applied the patch and also changed CPLStrtod() to fix reported behavior. I have read C standard carefully and found that current behavior is wrong and GNU/BSD strtod()s work in the right way. CPLStrtod() patched both in SVN HEAD and 1.4 branches.

Best regards, Andrey

Note: See TracTickets for help on using tickets.