Re: Review Request 127786: Remove custom read functions for QString and QStringList
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127786/ --- (Updated April 30, 2016, 1:50 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, David Faure and Milian Wolff. Changes --- Submitted with commit b4fea4e48455fa65c0ea530df30500a15bfd1452 by Jos van den Oever to branch master. Repository: kservice Description --- Writing KBuildSycoca is done with <<. Up till now there were special 'safe' functions for reading QString and QStringList. They only limited the size of QString and the number of allowed entries in QStringList. The cache file is created by the trusted system. If file size is an attack vector, these safe functions are useful and we should keep them. This patch is three commits: 1) Use the standard read function for reading QStringList 2) Use the standard read function for reading QString 3) Remove redundant #include ksycocaentry.h is included via kservice.h Diffs - src/CMakeLists.txt f4d09d5 src/services/kservicegroup.h c046314 src/services/kservicetypefactory.cpp 2edc57c src/sycoca/kctimefactory.cpp a8c7846 src/sycoca/ksycoca.cpp 5d43ef4 src/sycoca/ksycoca_p.h 119c3ee src/sycoca/ksycocaentry.cpp 5fbd158 src/sycoca/ksycocautils.cpp 84998b7 src/sycoca/ksycocautils_p.h aad9d50 Diff: https://git.reviewboard.kde.org/r/127786/diff/ Testing --- All tests still pass. Thanks, Jos van den Oever ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127786: Remove custom read functions for QString and QStringList
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127786/#review95037 --- Ship it! I see. That function in Qt3 was allocating memory right away, which is what our helper function was guarding against. With the new code in Qt4/Qt5 this is not necessary anymore. Please adjust the commit message before pushing, to talk about corrupted files rather than attacks. Thanks! - David Faure On April 29, 2016, 10:22 a.m., Jos van den Oever wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127786/ > --- > > (Updated April 29, 2016, 10:22 a.m.) > > > Review request for KDE Frameworks, David Faure and Milian Wolff. > > > Repository: kservice > > > Description > --- > > Writing KBuildSycoca is done with <<. Up till now there were special 'safe' > functions for reading QString and QStringList. They only limited the size of > QString and the number of allowed entries in QStringList. The cache file is > created by the trusted system. If file size is an attack vector, these safe > functions are useful and we should keep them. > > This patch is three commits: > > 1) Use the standard read function for reading QStringList > > > 2) Use the standard read function for reading QString > > > 3) Remove redundant #include > > ksycocaentry.h is included via kservice.h > > > Diffs > - > > src/CMakeLists.txt f4d09d5 > src/services/kservicegroup.h c046314 > src/services/kservicetypefactory.cpp 2edc57c > src/sycoca/kctimefactory.cpp a8c7846 > src/sycoca/ksycoca.cpp 5d43ef4 > src/sycoca/ksycoca_p.h 119c3ee > src/sycoca/ksycocaentry.cpp 5fbd158 > src/sycoca/ksycocautils.cpp 84998b7 > src/sycoca/ksycocautils_p.h aad9d50 > > Diff: https://git.reviewboard.kde.org/r/127786/diff/ > > > Testing > --- > > All tests still pass. > > > Thanks, > > Jos van den Oever > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127786: Remove custom read functions for QString and QStringList
> On apr 29, 2016, 10:56 a.m., David Faure wrote: > > This is not about trust and attacks, this is about not allocating 4 GB of > > RAM when reading a corrupted binary file. > > Jos van den Oever wrote: > That will only happen if the file or stream is 4 GB. `QDataStream > >>(QDataStream , QString )` allocates while reading in 1 MiB > chunks. http://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/tools/qstring.cpp#n8637 - Jos --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127786/#review95012 --- On apr 29, 2016, 10:22 a.m., Jos van den Oever wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127786/ > --- > > (Updated apr 29, 2016, 10:22 a.m.) > > > Review request for KDE Frameworks, David Faure and Milian Wolff. > > > Repository: kservice > > > Description > --- > > Writing KBuildSycoca is done with <<. Up till now there were special 'safe' > functions for reading QString and QStringList. They only limited the size of > QString and the number of allowed entries in QStringList. The cache file is > created by the trusted system. If file size is an attack vector, these safe > functions are useful and we should keep them. > > This patch is three commits: > > 1) Use the standard read function for reading QStringList > > > 2) Use the standard read function for reading QString > > > 3) Remove redundant #include > > ksycocaentry.h is included via kservice.h > > > Diffs > - > > src/CMakeLists.txt f4d09d5 > src/services/kservicegroup.h c046314 > src/services/kservicetypefactory.cpp 2edc57c > src/sycoca/kctimefactory.cpp a8c7846 > src/sycoca/ksycoca.cpp 5d43ef4 > src/sycoca/ksycoca_p.h 119c3ee > src/sycoca/ksycocaentry.cpp 5fbd158 > src/sycoca/ksycocautils.cpp 84998b7 > src/sycoca/ksycocautils_p.h aad9d50 > > Diff: https://git.reviewboard.kde.org/r/127786/diff/ > > > Testing > --- > > All tests still pass. > > > Thanks, > > Jos van den Oever > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127786: Remove custom read functions for QString and QStringList
> On apr 29, 2016, 10:56 a.m., David Faure wrote: > > This is not about trust and attacks, this is about not allocating 4 GB of > > RAM when reading a corrupted binary file. That will only happen if the file or stream is 4 GB. `QDataStream >>(QDataStream , QString )` allocates while reading in 1 MiB chunks. - Jos --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127786/#review95012 --- On apr 29, 2016, 10:22 a.m., Jos van den Oever wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127786/ > --- > > (Updated apr 29, 2016, 10:22 a.m.) > > > Review request for KDE Frameworks, David Faure and Milian Wolff. > > > Repository: kservice > > > Description > --- > > Writing KBuildSycoca is done with <<. Up till now there were special 'safe' > functions for reading QString and QStringList. They only limited the size of > QString and the number of allowed entries in QStringList. The cache file is > created by the trusted system. If file size is an attack vector, these safe > functions are useful and we should keep them. > > This patch is three commits: > > 1) Use the standard read function for reading QStringList > > > 2) Use the standard read function for reading QString > > > 3) Remove redundant #include > > ksycocaentry.h is included via kservice.h > > > Diffs > - > > src/CMakeLists.txt f4d09d5 > src/services/kservicegroup.h c046314 > src/services/kservicetypefactory.cpp 2edc57c > src/sycoca/kctimefactory.cpp a8c7846 > src/sycoca/ksycoca.cpp 5d43ef4 > src/sycoca/ksycoca_p.h 119c3ee > src/sycoca/ksycocaentry.cpp 5fbd158 > src/sycoca/ksycocautils.cpp 84998b7 > src/sycoca/ksycocautils_p.h aad9d50 > > Diff: https://git.reviewboard.kde.org/r/127786/diff/ > > > Testing > --- > > All tests still pass. > > > Thanks, > > Jos van den Oever > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127786: Remove custom read functions for QString and QStringList
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127786/#review95012 --- This is not about trust and attacks, this is about not allocating 4 GB of RAM when reading a corrupted binary file. - David Faure On April 29, 2016, 10:22 a.m., Jos van den Oever wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127786/ > --- > > (Updated April 29, 2016, 10:22 a.m.) > > > Review request for KDE Frameworks, David Faure and Milian Wolff. > > > Repository: kservice > > > Description > --- > > Writing KBuildSycoca is done with <<. Up till now there were special 'safe' > functions for reading QString and QStringList. They only limited the size of > QString and the number of allowed entries in QStringList. The cache file is > created by the trusted system. If file size is an attack vector, these safe > functions are useful and we should keep them. > > This patch is three commits: > > 1) Use the standard read function for reading QStringList > > > 2) Use the standard read function for reading QString > > > 3) Remove redundant #include > > ksycocaentry.h is included via kservice.h > > > Diffs > - > > src/CMakeLists.txt f4d09d5 > src/services/kservicegroup.h c046314 > src/services/kservicetypefactory.cpp 2edc57c > src/sycoca/kctimefactory.cpp a8c7846 > src/sycoca/ksycoca.cpp 5d43ef4 > src/sycoca/ksycoca_p.h 119c3ee > src/sycoca/ksycocaentry.cpp 5fbd158 > src/sycoca/ksycocautils.cpp 84998b7 > src/sycoca/ksycocautils_p.h aad9d50 > > Diff: https://git.reviewboard.kde.org/r/127786/diff/ > > > Testing > --- > > All tests still pass. > > > Thanks, > > Jos van den Oever > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 127786: Remove custom read functions for QString and QStringList
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127786/ --- Review request for KDE Frameworks, David Faure and Milian Wolff. Repository: kservice Description --- Writing KBuildSycoca is done with <<. Up till now there were special 'safe' functions for reading QString and QStringList. They only limited the size of QString and the number of allowed entries in QStringList. The cache file is created by the trusted system. If file size is an attack vector, these safe functions are useful and we should keep them. This patch is three commits: 1) Use the standard read function for reading QStringList 2) Use the standard read function for reading QString 3) Remove redundant #include ksycocaentry.h is included via kservice.h Diffs - src/CMakeLists.txt f4d09d5 src/services/kservicegroup.h c046314 src/services/kservicetypefactory.cpp 2edc57c src/sycoca/kctimefactory.cpp a8c7846 src/sycoca/ksycoca.cpp 5d43ef4 src/sycoca/ksycoca_p.h 119c3ee src/sycoca/ksycocaentry.cpp 5fbd158 src/sycoca/ksycocautils.cpp 84998b7 src/sycoca/ksycocautils_p.h aad9d50 Diff: https://git.reviewboard.kde.org/r/127786/diff/ Testing --- All tests still pass. Thanks, Jos van den Oever ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
