Created attachment 107480 [details] The problematic synctex file. Reloading (e.g., by pressing F5) a pdf file accompanied by the attached synctex file sometimes leads to segmentation faults of okular. The synctex file was produced by lualatex. Note that the attached pdf file does not correspond to the synctex file, but the problem also occurs with the pdf file that actually corresponds to the synctex file – I am just not comfortable with sharing its contents. Unfortunately I was not able to reproduce the behaviour with a synctex file from another document. Maybe bug 340619 is related. Version information: Help/About Okular: Version 1.2.0 Arch Linux package information: Name : okular Version : 17.08.0-1 Architecture : x86_64 I cannot remember when I first encountered the problem, but the problem was definitely already present with package version 17.04.3-1.
Created attachment 107481 [details] An arbitrary pdf file for testing.
Do i understand that you say that pressing and holding F5 with the file you attached in comment #1 will eventually crash okular for you? You wouldn't have a backtrace would you?
The infinite F5 crash is probably fixed by https://phabricator.kde.org/D7495 I'm undecided that that would fix anything that happens with "normal use" though.
Created attachment 107497 [details] gdb backtraces It is important that you put the files empty.synctex.gz and empty.pdf in the same folder and open empty.pdf with okular. You do not have to press and hold F5. For the backtrace, I only had to press F5 once. I initially discovered the problem when recompiling latex documents triggered the automatic reload. I noticed, I even get a segfault when I close okular (second backtrace). I have put the backtraces in an attachment. Sorry for the missing debug symbols. Maybe you can give me a hint how I get the missing symbols.
Reloading (F5) is implemented as 'closing and reopening' internally. So you probably see the same bug either way.
Confirmed with current git master. I'll attach a backtrace with debugging symbols.
Created attachment 107498 [details] Backtrace with debug symbols
Doesn't look like it's related to bug 340619 (which is about calligra, but we crash in synctex). Possible duplicate: bug 383980 (although there it crashes in _synctex_free_leaf(), while we crash in _synctex_free_input()) I can reproduce the bug with empty.synctex.gz and an ancient Okular 0.21.1 on KDE 4.14.4, too. Maybe there's something in luatex / TeXLive which changed recently (no reports since several years, now two within days)? Unfortunately this is not solved by D7495 (which is about trying to reload again when the previous reload is not finished yet), as the first crash (close Okular without pressing F5) segfaults in synctex without entering slotDoFileDirty(). Flupp: I don't think we need another backtrace, but if you are interested anyway, have a look at [1]. Seems like on Arch you'd need to recompile for yourself. [1] https://community.kde.org/Guidelines_and_HOWTOs/Debugging/How_to_create_useful_crash_reports#Arch
Proposal for fix: https://phabricator.kde.org/D7594 Updating synctex from 1.16 (6 years old) to the recent 1.19 fixes the crash. As this is a huge upgrade, please also run through extensive tests of the synctex functionality with your local documents after applying the fix, so we don't introduce regressions.
*** Bug 383980 has been marked as a duplicate of this bug. ***
Created attachment 107745 [details] Arch Linux PKGBUILD for version 17.08.0 that applys the patch The patch fixes the problem for me. I am using okular with your patch applied to version 16fd5686cce4f760f6c85b9ae618057dc3ed070c (cf. attachment) daily since 2017-08-29 and have not experienced any problems so far.
Git commit bd20e48c3c8c82ed11ae73aae0467c8ef49fca3b by Henrik Fehlauer. Committed on 12/09/2017 at 19:56. Pushed by rkflx into branch 'master'. Update to synctex 1.19 Summary: This should prevent crashes when reloading some synctex-enabled pdf files created with newer versions of TeXLive. We also gain bugfixes, features and improved accuracy from the last 6 years of synctex development. Procedure followed: - svn co svn://tug.org/texlive/trunk/Build/source/texk/web2c/synctexdir - Check out revision 45150 - Update files present in core/synctex/* - Adapt Okular code to changes - Review and drop or update/apply old patches using quilt - Create missing patches for local synctex changes - New patch: Omit warning message when opening non-synctex pdf - Two new patches to fix more compiler warnings - New patch: Plug multiple leaks and prevent a segfault TODO for later: - Move sync file detection code to Okular to never call into synctex C code for non-synctex files - Evaluate feasibility of upstreaming all patches for TeXLive 2018 and using synctex as a library FIXED-IN: 17.12.0 Test Plan: - No crash in synctex on reloading empty.pdf from bugreport anymore. - Shift-clicking on a word in a simple pdf opens Kate with the corresponding tex line. - Forward and backward search in Kile seems to work. - Works with synctex files from both TeXLive 2015 and 2017. - PartTest::testForwardPDF still passes. - No additional memory leaks in autotests and with basic synctex and non-synctex usage of Okular. Reviewers: #okular, sander, #kile, aacid Reviewed By: #okular, aacid Subscribers: mludwig, aacid Tags: #okular Differential Revision: https://phabricator.kde.org/D7594 M +4 -4 autotests/parttest.cpp M +5 -5 core/document.cpp M +1 -1 core/document_p.h M +9 -8 core/synctex/patches/00-disable-SYNCTEX_INLINE.diff D +0 -42 core/synctex/patches/01-fix-win32-define.diff M +51 -12 core/synctex/patches/04-gcc-specify-printf-format.diff D +0 -13 core/synctex/patches/05-fix-error-formats.diff M +10 -9 core/synctex/patches/06-mingw-_synctex_error.diff D +0 -21 core/synctex/patches/07-synctex_scanner_new_with_output_file-reset-mode.diff M +29 -23 core/synctex/patches/08-fix_cpp_comments.diff D +0 -22 core/synctex/patches/09-fix_path_comparison.diff A +17 -0 core/synctex/patches/10-fix-typo.diff A +49 -0 core/synctex/patches/11-fix-unused-parameters-warnings.diff A +17 -0 core/synctex/patches/12-omit-no-file-warning.diff A +326 -0 core/synctex/patches/13-fix-Wundef-warnings.diff A +62 -0 core/synctex/patches/14-fix-misc-compiler-warnings.diff A +54 -0 core/synctex/patches/15-prevent-leaks-and-segfault.diff M +6 -4 core/synctex/patches/series M +8471 -4038 core/synctex/synctex_parser.c M +410 -332 core/synctex/synctex_parser.h A +552 -0 core/synctex/synctex_parser_advanced.h [License: BSD X11 (BSD like)] M +1 -1 core/synctex/synctex_parser_local.h A +246 -0 core/synctex/synctex_parser_readme.md D +0 -141 core/synctex/synctex_parser_readme.txt M +176 -92 core/synctex/synctex_parser_utils.c M +39 -19 core/synctex/synctex_parser_utils.h M +1 -1 core/synctex/synctex_parser_version.txt https://commits.kde.org/okular/bd20e48c3c8c82ed11ae73aae0467c8ef49fca3b
Created attachment 109116 [details] old synctex crashing okular 17.12 Unfortunately, okular might crash with old synctex files now. A problematic synctex file is attached. Note: Okular does not always crash with this file. I will attach a backtrace of a crashing trial and the output of a non-crashing trial. Used command: gdb --ex run --ex backtrace --ex 'set confirm off' --ex quit --args okular empty.pdf |& tee "$(date -Is)" Okular version: branch Applications/17.12 commit 02a4cfb26afc822777377c9539473f757b277490 I guess, this is not really a bug in okular, but in the synctex parser you just use. Please tell me, if I shall report this bug again for the synctex project. Also, I start to seriously doubt the code quality of the synctex parser. Assuming the code quality is really bad, I could imagine that the unconditional loading of synctex files together with pdfs poses a security risk. However, I do not know how okular should deal with this.
Created attachment 109117 [details] old synctex crashing okular 17.12 – backtrace of crashing trial
Created attachment 109118 [details] old synctex crashing okular 17.12 – output of non-crashing trial
Thanks for reaching out, this looks pretty bad. Can confirm the crash in Okular is introduced with the commit above. It seems even if it was tested with older versions of TeXLive this slipped through, sorry for that :\ As for the quality of the upstream code: Yeah, it has issues as hinted at in the TODO in the commit message above. Project/code/branch management, crossplatform building and (non-existing) buildsystem also leave much to be desired. In the medium term we should address two goals (besides getting rid of our fork): - Do not call into synctex code for non-synctex PDFs. - Figure out how to handle this attack: Users gets sent exploit.zip, containing exploit.pdf and exploit.synctex.gz. Game over after clicking on the PDF. Ideas: - Disable synctex entirely. → This would be the most responsible thing to do, but distros might patch it in anyway… - Warning message. → Does not help, just look at all the MS Office macro viruses. - Option in Okulars' preferences. → Helps "regular" users, but not those actually needing synctex. - Improve synctex code, e.g. with static analysis, running fuzzers, … → Would need some helping hands. - Deploy seccomp profiles for Okular. → Would need help too, but also alleviates attacks against other formats. Most likely nothing will happen due to lack of manpower though, I fear. Feel free to pitch this to anyone interested. Regarding fixing things right now, I have identified two issues (see next comment for the details): - In some situations, the synctex upstream code shows parse errors. - The parse error triggers a code path where one of our hardening patches now falls over. I think I can fix the second problem (will be either in the RC tomorrow, but latest for the final release). Flupp: Could you report the first problem over at https://github.com/jlaurens/synctex/issues?
ISSUE 1: Parse error in synctex 1.19 upstream code Here is what I did: Tried to make sense of where the actual upstream code is (did not succeed): git svn clone svn://tug.org/texlive/trunk/Build/source/texk/web2c/synctexdir git clone https://github.com/jlaurens/synctex Figured out how to build this code: gcc -D__SYNCTEX_WORK__ -I. synctex_main.c synctex_parser.c synctex_parser_utils.c -lz -o synctex (some do not build at all, some need the "__" removed) Figured out how to test: ./synctex view -i 1:1:00_00_dissertation.tex -o empty-august.pdf 1>/dev/null ./synctex view -i 1:1:main.tex -o empty-november.pdf 1>/dev/null Did some bisecting: GitHub: - fcbfa2c20891 fixes the crash from August, I'm positive we have this one in the SVN version Okular pulled in. - The November file does not want to crash this way. However, we get "SyncTeX ERROR: Ignored record", incidentally and sadly also since fcbfa2c20891. TUG: - August file: @44795 fixes the crash, as expected. We got this in Okular. - November file: Since @44795 we get "! SyncTeX Error : Ignored record ...", but no crash again. With gdb Okular does crash and we get "! SyncTeX Error :", but "Ignored record" is only visible once poking at the "reason" variable. fcbfa2c20891 is titled "Nested sheet patch and other stuff", unfortunately it is a massive code dump so we cannot see why it fixes the crash in the August file and causes the error in the November file. @44795 seems to contain fcbfa2c20891 together with even more changes. Conclusion: Should be reported upstream, upgrade to the newest synctex version once fixed there and rebase all patches :( ISSUE 2: 04-gcc-specify-printf-format.diff While ./synctex throws the error without crashing, Okular shows part of the error but then crashes when trying to print the rest. It did not crash before because the documents tested caused no error needing to be logged. It crashes when accessing "arg" in vfprintf, because the patch to avoid "function ‘_synctex_log’ might be a candidate for ‘gnu_printf’ format attribute" was adapted/extended wrongly. No idea why sometimes it does not crash for Flupp, perhaps sheer luck with some pointer access. Conclusion: Either move some things around regarding "va_list" (i.e. add va_start and va_end), or revert to the previous state and live with the compiler warning for now.
Git commit dccd83783d145409e22d822ec2b0a3645f549fa3 by Henrik Fehlauer. Committed on 30/11/2017 at 13:57. Pushed by rkflx into branch 'Applications/17.12'. Prevent Okular from crashing when synctex logs an error bd20e48c3c8c updated Okular's copy of the synctex code to 1.19. Unfortunately since this version the upstream code logs errors like `"! SyncTeX Error : Ignored record...` when accessing selected synctex files created with older versions of synctex. The upstream `_synctex_log` contains `va_list arg` as a parameter, but fails to initialize and tear down this properly via `va_start` and `va_end`. In general this seems to work for the single argument case. However, once we apply our hardening patch to get rid of the `gnu_printf format attribute` warning and thus introduce a variadic argument, things go wrong. To fix this, we add the missing code. The remaining changes are just refreshing the patches. Test Plan: Opening `empty.pdf` with `empty.synctex.gz` from https://bugs.kde.org/attachment.cgi?id=109116 located in the same folder does not lead to Okular segfaulting anymore. M +11 -1 core/synctex/patches/04-gcc-specify-printf-format.diff M +1 -1 core/synctex/patches/06-mingw-_synctex_error.diff M +1 -1 core/synctex/patches/08-fix_cpp_comments.diff M +1 -1 core/synctex/patches/10-fix-typo.diff M +1 -1 core/synctex/patches/13-fix-Wundef-warnings.diff M +2 -0 core/synctex/synctex_parser_utils.c https://commits.kde.org/okular/dccd83783d145409e22d822ec2b0a3645f549fa3
Created attachment 109133 [details] recursively search for pdf with synctex in current dir and open in okular The commit in comment 18 seems to fix all my synctex-related crash problems. In fact, I have opened every pdf accompanied by a synctex file I could find in my home dir (cf. attached script) and no error occurred (246 files tested). For some files I got several messages of the form ! SyncTeX Error : Ignored record ...>(line ${some_integer}) @comment 16: > Ideas: I have another idea for working around issues with synctex: Okular could just call the synctex command line tool with the edit subcommand as soon as the user shift-clicks. While this is still not a perfect solution, it has the following advantages: * Okular does not have to know anything about synctex – it does not even have to check, if there is a synctex file, although this would be more user friendly. * Loading of the synctex file (and therefore execution of a potential exploit) is delayed until the user really asks for synctex by shift-clicking. * If the synctex cmdline tool crashes, it does not affect okular (but now the synctex cndline tool might be exploited, of course). * After the first shift-click, the user could be asked for confirmation, if he trusts the synctex file. * Okular would not depend on synctex. Okular would work just fine, if the synctex cmdline tool is not installed. Delaying to load the synctex file until the first shift-click is of course also an option if synctex is used as library. > - In some situations, the synctex upstream code shows parse errors. > > Flupp: Could you report [this] first problem over at https://github.com/jlaurens/synctex/issues? Are you sure, this is not an expected behavior for old synctex files? @comment 17: > No idea why sometimes it does not crash for Flupp, perhaps sheer luck with some pointer access. I’d guess the same. The guess is supported by the random characters appearing in the output.
Thanks for testing, your collection sounds impressive! You should run your script regularly and report any new problems ;) As for Okular, we (as in whoever contributes this) should extend https://phabricator.kde.org/source/okular/browse/master/autotests/parttest.cpp;069a18b041ce3615ac9c6546c57399fe61481dc8$183 to also cover broken documents. Apart from that, it's not really Okular's job to test synctex itself. There is also <https://github.com/jlaurens/synctex/tree/2017/synctex test files>, but this is nearly unusable without any buildsystem/testrunner. I wonder if upstream is running all those test manually? > Are you sure, this is not an expected behavior for old synctex files? Well, screaming "! Error" and "ignoring records" is not something I would expect ;) It was my first guess too yesterday, but looking at the upstream commit you'll see that this line was there before (but commented out), it was only moved around and set active by default. This means the format errors could have been there all along. Are they going away if you recompile those old documents? Still, it looks like broken backwards compatibility. Anyway, the only way to know for sure is to ask upstream, because I don't know a thing about this and also I have no access to the tex source to create the synctex file in question (with an older texlive version?) in the first place… Worst that could happen is a WONTFIX, best a patch. Just try it, I'd say. > Okular could just call the synctex command line tool Interesting, might be worth thinking about. However, I'm not yet convinced switching from a C API (with type checking and only breaking at compile time, i.e. before reaching users) to a Bash API (passing strings over pipes and runtime version checks breaking on user's machines) is a good idea. > If the synctex cmdline tool crashes, it does not affect okular As you noted, for exploits this point is irrelevant. For crashing in general, decoupling is only worth the effort if it crashes very often, but again, then the crash should be fixed in the first place as we have access to the source. > After the first shift-click, the user could be asked for confirmation Not sure about this one. I think this approach has been shown to fail where it is needed most: Non-technical users. For all others it is just an annoyance. Would you like Gwenview to ask whether you trust a JPG? (We have dozens of duplicate EXIV crasher bugs, but only two for synctex.) > Okular would not depend on synctex. Could also be achieved by runtime-loading synctex support as a plugin? > Delaying to load the synctex file until the first shift-click is of course > also an option if synctex is used as library. This is an excellent idea! If only I had more time to work on such things…