D20877: Fix exivextractor crash with malformed files
astippich created this revision. Herald added projects: Frameworks, Baloo. Herald added subscribers: Baloo, kde-frameworks-devel. astippich requested review of this revision. REVISION SUMMARY Prevent a segfault by explicitly checking everywhere for valid entries. BUG: 405210 REPOSITORY R286 KFileMetaData BRANCH fix_exiv_crash REVISION DETAIL https://phabricator.kde.org/D20877 AFFECTED FILES src/extractors/exiv2extractor.cpp To: astippich Cc: kde-frameworks-devel, #baloo, gennad, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D20877: Fix exivextractor crash with malformed files
astippich added a reviewer: bruns. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D20877 To: astippich, bruns Cc: kde-frameworks-devel, #baloo, gennad, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D20877: Fix exivextractor crash with malformed files
bruns added inline comments. INLINE COMMENTS > exiv2extractor.cpp:299 > Exiv2::ExifData::const_iterator it = > data.findKey(Exiv2::ExifKey("Exif.GPSInfo.GPSAltitude")); > -if (it != data.end() && (it->value().typeId() == Exiv2::unsignedRational > || it->value().typeId() == Exiv2::signedRational)) { > +if (it != data.end() && it->count() > 0 && > +(it->value().typeId() == Exiv2::unsignedRational || > it->value().typeId() == Exiv2::signedRational)) { `!it->empty()` ? REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D20877 To: astippich, bruns Cc: kde-frameworks-devel, #baloo, gennad, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D20877: Fix exivextractor crash with malformed files
astippich added inline comments. INLINE COMMENTS > bruns wrote in exiv2extractor.cpp:299 > `!it->empty()` ? Unfortunately, empty() does not exist. Same was done in https://phabricator.kde.org/D16165 REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D20877 To: astippich, bruns Cc: kde-frameworks-devel, #baloo, gennad, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D20877: Fix exivextractor crash with malformed files
bruns added inline comments. INLINE COMMENTS > astippich wrote in exiv2extractor.cpp:299 > Unfortunately, empty() does not exist. Same was done in > https://phabricator.kde.org/D16165 Yes, probably as `count()` is O(1). > exiv2extractor.cpp:303 > it = data.findKey(Exiv2::ExifKey("Exif.GPSInfo.GPSAltitudeRef")); > if ((ratio.second != 0) && (it != data.end()) && > (it->value().typeId() == Exiv2::unsignedByte || it->value().typeId() == > Exiv2::signedByte)) { > auto altRef = it->value().toLong(); Shouldn't there be a check here as well? And probably move the `ratio.second` to after its assignment and immediately return. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D20877 To: astippich, bruns Cc: kde-frameworks-devel, #baloo, gennad, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D20877: Fix exivextractor crash with malformed files
astippich updated this revision to Diff 57135. astippich added a comment. - also check for altitude ref REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D20877?vs=57130&id=57135 BRANCH fix_exiv_crash REVISION DETAIL https://phabricator.kde.org/D20877 AFFECTED FILES src/extractors/exiv2extractor.cpp To: astippich, bruns Cc: kde-frameworks-devel, #baloo, gennad, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D20877: Fix exivextractor crash with malformed files
astippich marked an inline comment as done. astippich added inline comments. INLINE COMMENTS > bruns wrote in exiv2extractor.cpp:303 > Shouldn't there be a check here as well? And probably move the > `ratio.second` to after its assignment and immediately return. Yep, forgot REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D20877 To: astippich, bruns Cc: kde-frameworks-devel, #baloo, gennad, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D20877: Fix exivextractor crash with malformed files
bruns added a comment. Otherwise, LGTM INLINE COMMENTS > astippich wrote in exiv2extractor.cpp:303 > Yep, forgot Can you add a line break here as well? REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D20877 To: astippich, bruns Cc: kde-frameworks-devel, #baloo, gennad, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D20877: Fix exivextractor crash with malformed files
astippich updated this revision to Diff 57157. astippich marked an inline comment as done. astippich added a comment. - add line break REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D20877?vs=57135&id=57157 BRANCH fix_exiv_crash REVISION DETAIL https://phabricator.kde.org/D20877 AFFECTED FILES src/extractors/exiv2extractor.cpp To: astippich, bruns Cc: kde-frameworks-devel, #baloo, gennad, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D20877: Fix exivextractor crash with malformed files
astippich marked 2 inline comments as done. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D20877 To: astippich, bruns Cc: kde-frameworks-devel, #baloo, gennad, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D20877: Fix exivextractor crash with malformed files
bruns accepted this revision. This revision is now accepted and ready to land. REPOSITORY R286 KFileMetaData BRANCH fix_exiv_crash REVISION DETAIL https://phabricator.kde.org/D20877 To: astippich, bruns Cc: kde-frameworks-devel, #baloo, gennad, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D20877: Fix exivextractor crash with malformed files
ngraham accepted this revision. REPOSITORY R286 KFileMetaData BRANCH fix_exiv_crash REVISION DETAIL https://phabricator.kde.org/D20877 To: astippich, bruns, ngraham Cc: kde-frameworks-devel, #baloo, gennad, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D20877: Fix exivextractor crash with malformed files
This revision was automatically updated to reflect the committed changes. Closed by commit R286:e227a7ce4587: Fix exivextractor crash with malformed files (authored by astippich). REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D20877?vs=57157&id=57212 REVISION DETAIL https://phabricator.kde.org/D20877 AFFECTED FILES src/extractors/exiv2extractor.cpp To: astippich, bruns, ngraham Cc: kde-frameworks-devel, #baloo, gennad, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams