Version: (using KDE KDE 3.4.0) Installed from: Gentoo Packages OS: Linux It seems that no limits (e.g. timeout) are put on latex. The result is a very easy Denial of Service attack: just send some code that results in an infinite loop. For example $$\def\foo{\foo} \foo$$ is an easy way to kill the remote kopete (and the local one too, if the plugin is active). OK, it's not really killed, it's just waiting indefinitely... Simply killing the latex process allows kopete to recover, but still, this should be fixed somehow. I'm wondering what else could be done by arbitrary LaTeX code. Filling up free disk space comes to my mind. What's the working directory when latex is called? (latex can read, write and create files in its working directory.) But apart from the security issues, I quite like it! Thanks!
.. the issue of arbitrary LaTeX code seems worrisome..
Should i blacklist some commands ? Which commands ?
On Friday 29 April 2005 21:11, Olivier Goffart wrote: > Should i blacklist some commands ? Yes, probably. Unfortunately LaTeX is quite a complicated beast. > Which commands ? IMO there are the following general categories of commands that shouldn't be allowed in KopeTeX: * programming commands. Examples: \def, \let, \futurelet, \newcommand, \renewcommand, \if..., \else, \fi. * writing files: \write * reading files: \input, \include * tinkering with the parser: \chardef, \catcode, \makeatletter, \noexpand, \toksdef. Please note that I'm not a TeXpert, and that these are just the commands I could think of right now. (And I'm not even sure they are all dangerous. It's just that they have no reason to be allowed and could be exploited in non-obvious ways.) There are probably many more commands that need to be filtered. IMHO it would be a good idea to actively monitor the latex process and kill it if it takes too long, or if it somehow manages to write more than 1 MiB to disk. A combination of filtering and monitoring should make it secure. IIRC latex won't write any files outside its working directory, so it should always be run in a newly created temporary directory. (But I think that's already the case.) If possible, it would also be a good idea to restrict the plugin to contacts that are on the contact list.
Created an attachment (id=10976) [details] patch that blacklist some commands. here is the patch i'm about to commit. i've googled a bit, and i find some list of command to blacklist. I hope this is enough.
On Tuesday 10 May 2005 16:38, Olivier Goffart wrote: > i've googled a bit, and i find some list of command to blacklist. I hope > this is enough. In fact there is not a single "if" command. There's \if, \ifcase, \ifcat, \ifdim, \ifeof, \iffalse, \ifhbox, \ifhmode, \ifinner, \ifmmode, \ifnum, \ifodd, \iftrue, \ifundefined, \ifvbox, \ifvmode, \ifvoid, \ifx. At least these are the ones that are listed in the TeXbook index. But AFAIK they all need to be ended by a \fi, so in theory it should be sufficient to just block that one.
On Tuesday 10 May 2005 16:38, Olivier Goffart wrote: > i've googled a bit, and i find some list of command to blacklist. I hope > this is enough. I just found another \def like command that is defined by LaTeX: \DeclareRobustCommand
SVN commit 412367 by ogoffart: Security: blacklist some latex command that may cause trouble. CCBUG: 103026 M +15 -6 trunk/KDE/kdenetwork/kopete/plugins/latex/latexplugin.cpp M +5 -0 trunk/KDE/kdenetwork/kopete/plugins/latex/latexplugin.h --- trunk/KDE/kdenetwork/kopete/plugins/latex/latexplugin.cpp #412366:412367 @@ -113,12 +113,14 @@ if (pos >= 0 ) { QString match = rg.cap(0); -// kdDebug() << k_funcinfo << " captured: " << match << endl; + pos += rg.matchedLength(); + QString formul=match; + if(!securityCheck(formul)) + continue; + QString fileName=handleLatex(formul.replace("$$","")); -// kdDebug() << k_funcinfo << " render process finished..." << endl; - // get the image and encode it with base64 #if ENCODED_IMAGE_MODE QImage renderedImage( fileName ); @@ -134,9 +136,6 @@ #else replaceMap[Kopete::Message::escape(match)] = fileName; #endif - // ok, go for the next one - pos += rg.matchedLength(); - } } @@ -178,6 +177,8 @@ if( rg.search(messageText) != -1 ) { QString latexFormula = rg.cap(1); + if(!securityCheck( latexFormula )) + return; QString url = handleLatex(latexFormula); @@ -217,7 +218,15 @@ return fileName; } +bool LatexPlugin::securityCheck(const QString &latexFormula) +{ + return !latexFormula.contains(QRegExp("\\\\(def|let|futurelet|newcommand|renewcomment|if[a-z]*|else|fi|write|input|include" + "|chardef|catcode|makeatletter|noexpand|toksdef|every|errhelp|errorstopmode|scrollmode|nonstopmode|batchmode" + "|read|csname|newhelp|relax|afterground|afterassignment|expandafter|noexpand|special|command|loop|repeat|toks" + "|output|line|mathcode|name|item|section|mbox|DeclareRobustCommand)[^a-zA-Z]")); +} + void LatexPlugin::slotSettingsChanged() { LatexConfig::self()->readConfig(); --- trunk/KDE/kdenetwork/kopete/plugins/latex/latexplugin.h #412366:412367 @@ -59,6 +59,11 @@ * gives a latex formula, and return the filename of the file where the latex is stored. */ QString handleLatex(const QString &latex); + + /** + * return false if the latex formula may contains malicious commands + */ + bool securityCheck(const QString & formula); static LatexPlugin* s_pluginStatic; QString m_convScript;
SVN commit 412368 by ogoffart: Backport: Security: blacklist some latex command that may cause trouble. BUG: 103026 M +15 -6 branches/KDE/3.4/kdenetwork/kopete/plugins/latex/latexplugin.cpp M +5 -0 branches/KDE/3.4/kdenetwork/kopete/plugins/latex/latexplugin.h --- branches/KDE/3.4/kdenetwork/kopete/plugins/latex/latexplugin.cpp #412367:412368 @@ -113,12 +113,14 @@ if (pos >= 0 ) { QString match = rg.cap(0); -// kdDebug() << k_funcinfo << " captured: " << match << endl; + pos += rg.matchedLength(); + QString formul=match; + if(!securityCheck(formul)) + continue; + QString fileName=handleLatex(formul.replace("$$","")); -// kdDebug() << k_funcinfo << " render process finished..." << endl; - // get the image and encode it with base64 #if ENCODED_IMAGE_MODE QImage renderedImage( fileName ); @@ -134,9 +136,6 @@ #else replaceMap[Kopete::Message::escape(match)] = fileName; #endif - // ok, go for the next one - pos += rg.matchedLength(); - } } @@ -178,6 +177,8 @@ if( rg.search(messageText) != -1 ) { QString latexFormula = rg.cap(1); + if(!securityCheck( latexFormula )) + return; QString url = handleLatex(latexFormula); @@ -217,7 +218,15 @@ return fileName; } +bool LatexPlugin::securityCheck(const QString &latexFormula) +{ + return !latexFormula.contains(QRegExp("\\\\(def|let|futurelet|newcommand|renewcomment|if[a-z]*|else|fi|write|input|include" + "|chardef|catcode|makeatletter|noexpand|toksdef|every|errhelp|errorstopmode|scrollmode|nonstopmode|batchmode" + "|read|csname|newhelp|relax|afterground|afterassignment|expandafter|noexpand|special|command|loop|repeat|toks" + "|output|line|mathcode|name|item|section|mbox|DeclareRobustCommand)[^a-zA-Z]")); +} + void LatexPlugin::slotSettingsChanged() { LatexConfig::self()->readConfig(); --- branches/KDE/3.4/kdenetwork/kopete/plugins/latex/latexplugin.h #412367:412368 @@ -59,6 +59,11 @@ * gives a latex formula, and return the filename of the file where the latex is stored. */ QString handleLatex(const QString &latex); + + /** + * return false if the latex formula may contains malicious commands + */ + bool securityCheck(const QString & formula); static LatexPlugin* s_pluginStatic; QString m_convScript;
On Wednesday 11 May 2005 16:05, Olivier Goffart wrote: > !latexFormula.contains(QRegExp("\\\\(def|let|futurelet|newcommand|renewcomm >ent|if[a-z]*|else|fi|write|input|include" > + "|chardef|catcode|makeatletter|noexpand|toksdef|every|errhelp|errorstop If I understand that regexp correctly it will also match \iff. Please consider changing this behaviour, since \iff is an often used command for "if and only if" (displayed as <=>). Thanks for taking the time to fix this issue.
SVN commit 412410 by ogoffart: Don't blacklist \iff CCBUG: 103026 M +23 -23 trunk/KDE/kdenetwork/kopete/plugins/latex/latexplugin.cpp
You need to log in before you can comment on or make changes to this bug.