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
