D15826: [Balooshow] Avoid out-of-bounds access when accessing corrupt db data

This revision was automatically updated to reflect the committed changes. Closed by commit R293:563dd3418b60: [Balooshow] Avoid out-of-bounds access when accessing corrupt db data (authored by bruns). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE

D15826: [Balooshow] Avoid out-of-bounds access when accessing corrupt db data

poboiko accepted this revision. poboiko added a comment. This revision is now accepted and ready to land. Yep, fine by me REPOSITORY R293 Baloo BRANCH oob2 REVISION DETAIL https://phabricator.kde.org/D15826 To: bruns, #baloo, #frameworks, poboiko Cc: anthonyfieroni,

D15826: [Balooshow] Avoid out-of-bounds access when accessing corrupt db data

bruns marked 3 inline comments as done. bruns added a comment. @poboiko - good to go? REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D15826 To: bruns, #baloo, #frameworks, poboiko Cc: anthonyfieroni, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun,

D15826: [Balooshow] Avoid out-of-bounds access when accessing corrupt db data

bruns marked 11 inline comments as done. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D15826 To: bruns, #baloo, #frameworks, poboiko Cc: anthonyfieroni, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams

D15826: [Balooshow] Avoid out-of-bounds access when accessing corrupt db data

bruns updated this revision to Diff 44224. bruns added a comment. use KLocalizedString for deferred i18n substitution REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15826?vs=43176=44224 BRANCH oob2 REVISION DETAIL https://phabricator.kde.org/D15826

D15826: [Balooshow] Avoid out-of-bounds access when accessing corrupt db data

poboiko added a comment. I'm not entirely sure how translating system works, but won't it pop up as 4 identical lines in i.e. `Lokalize`, causing frustration to our translators? It would also mean that if we would want to change the message, we would have to do it in 4 different lines.

D15826: [Balooshow] Avoid out-of-bounds access when accessing corrupt db data

bruns updated this revision to Diff 43176. bruns added a comment. make (introduction of) error message translatable REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15826?vs=42547=43176 BRANCH oob REVISION DETAIL https://phabricator.kde.org/D15826

D15826: [Balooshow] Avoid out-of-bounds access when accessing corrupt db data

bruns added a comment. In D15826#339367 , @poboiko wrote: > BTW, does it also wipe tags and other user-provided metadata? Tags and comments are store in XAttrs, i.e. with the file. REPOSITORY R293 Baloo REVISION DETAIL

D15826: [Balooshow] Avoid out-of-bounds access when accessing corrupt db data

poboiko added inline comments. INLINE COMMENTS > bruns wrote in main.cpp:211 > but you are not allowed to access `word[2]` if `word.length() < 3`. We don't access it directly, and `indexOf` performs internal checks. For example, `QStringLiteral("ab").indexOf('c', 5)` seem to be perfectly valid

D15826: [Balooshow] Avoid out-of-bounds access when accessing corrupt db data

bruns added inline comments. INLINE COMMENTS > poboiko wrote in main.cpp:211 > But still, if the term is short (namely, length < 4), we will either won't > have "-" (this corresponds to `posOfNonNumeric < 0`, and that's > `X`), or it will be the last symbol (something like `X1-` - which is >

D15826: [Balooshow] Avoid out-of-bounds access when accessing corrupt db data

poboiko added a comment. Maybe we should print also a suggestion to user, something like (maybe rephrase it better) WARNING: Looks like your index is corrupted. We suggest you to run `balooctl disable && balooctl disable` to wipe it and rebuild from scratch so they won't

D15826: [Balooshow] Avoid out-of-bounds access when accessing corrupt db data

bruns added inline comments. INLINE COMMENTS > poboiko wrote in main.cpp:204 > I'm not sure this check is needed - the other one (`posOfNonNumeric < 0`) > seem to be covering this case. > There shouldn't be an empty `QByteArray`, right? See `word[0]` access directly after. I have fixed to many

D15826: [Balooshow] Avoid out-of-bounds access when accessing corrupt db data

poboiko added a comment. I never experienced such corruption, though, but sanity check shouldn't hurt. INLINE COMMENTS > main.cpp:204 > +if (arr.length() <= 1) { > +stream << "Malformed term (short): " << arr << "\n"; > +continue; I'm

D15826: [Balooshow] Avoid out-of-bounds access when accessing corrupt db data

bruns added a reviewer: poboiko. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D15826 To: bruns, #baloo, #frameworks, poboiko Cc: anthonyfieroni, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams

D15826: [Balooshow] Avoid out-of-bounds access when accessing corrupt db data

bruns added inline comments. INLINE COMMENTS > anthonyfieroni wrote in main.cpp:217 > posOfNonNumeric < 0 or == -1 ? Thou shall not code after bedtime ... Thx. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D15826 To: bruns, #baloo, #frameworks Cc: anthonyfieroni,

D15826: [Balooshow] Avoid out-of-bounds access when accessing corrupt db data

bruns updated this revision to Diff 42547. bruns marked an inline comment as done. bruns added a comment. fix "not found" condition for `word.indexOf(...)` REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15826?vs=42530=42547 BRANCH db_robustness2 REVISION

D15826: [Balooshow] Avoid out-of-bounds access when accessing corrupt db data

anthonyfieroni added inline comments. INLINE COMMENTS > main.cpp:217 > +int posOfNonNumeric = word.indexOf('-', 2); > +if ((posOfNonNumeric < -1) || ((posOfNonNumeric + 1) > == word.length())) { > +stream << "Malformed

D15826: [Balooshow] Avoid out-of-bounds access when accessing corrupt db data

bruns created this revision. bruns added reviewers: Baloo, Frameworks. Herald added projects: Frameworks, Baloo. Herald added a subscriber: kde-frameworks-devel. bruns requested review of this revision. REVISION SUMMARY Looping over `word` without bounds check may cause illegal memory accesses,