]> git.vanrenterghem.biz Git - git.ikiwiki.info.git/blobdiff - doc/plugins/po.mdwn
po/todo(security): many research results
[git.ikiwiki.info.git] / doc / plugins / po.mdwn
index 39575fb63266d6cc0913999032aa9cd614256955..e88cc3106531fd301ed99a5f2c58bddc24b91c8e 100644 (file)
@@ -6,6 +6,8 @@ gettext, using [po4a](http://po4a.alioth.debian.org/).
 
 It depends on the Perl `Locale::Po4a::Po` library (`apt-get install po4a`).
 
+[[!toc levels=2]]
+
 Introduction
 ============
 
@@ -215,30 +217,131 @@ TODO
 Security checks
 ---------------
 
-- Can any sort of directives be put in po files that will
-  cause mischief (ie, include other files, run commands, crash gettext,
-  whatever). The [PO file
-  format](http://www.gnu.org/software/gettext/manual/gettext.html#PO-Files)
-  should contain the answer.
-- Any security issues on running po4a on untrusted content?
-
 ### Security history
 
-#### GNU gettext
-- [CVE-2004-0966](http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2004-0966)
-  / [Debian bug #278283](http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=278283):
+The only past security issues I could find in GNU gettext and po4a
+are:
+
+- [CVE-2004-0966](http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2004-0966),
+  *i.e.* [Debian bug #278283](http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=278283):
   the autopoint and gettextize scripts in the GNU gettext package
   1.14 and later versions, as used in Trustix Secure Linux 1.5
   through 2.1 and other operating systems, allows local users to
   overwrite files via a symlink attack on temporary files.
-
-#### po4a
--
-  [CVE-2007-4462](http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2007-4462):
-  lib/Locale/Po4a/Po.pm in po4a before 0.32 allows local users to
+- [CVE-2007-4462](http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2007-4462):
+  `lib/Locale/Po4a/Po.pm` in po4a before 0.32 allows local users to
   overwrite arbitrary files via a symlink attack on the
   gettextization.failed.po temporary file.
 
+**FIXME**: check whether this plugin would have been a possible attack
+vector to exploit these vulnerabilities.
+
+Depending on my mood, the lack of found security issues can either
+indicate that there are none, or reveal that no-one ever bothered to
+find (and publish) them.
+
+### PO file features
+
+Can any sort of directives be put in po files that will cause mischief
+(ie, include other files, run commands, crash gettext, whatever)?
+
+> No [documented](http://www.gnu.org/software/gettext/manual/gettext.html#PO-Files)
+> directive is supposed to do so. [[--intrigeri]]
+
+### Running po4a on untrusted content
+
+Are there any security issues on running po4a on untrusted content?
+
+To say the least, this issue is not well covered, at least publicly:
+
+- the documentation does not talk about it;
+- grep'ing the source code for `security` or `trust` gives no answer.
+
+On the other hand, a po4a developer answered my questions in
+a convincing manner, stating that processing untrusted content was not
+an initial goal, and analysing in detail the possible issues.
+
+#### Already checked
+
+- the core (`Po.pm`, `Transtractor.pm`) should be safe
+- po4a source code was fully checked for other potential symlink
+  attacks, after discovery of one such issue
+- the only external program run by the core is `diff`, in `Po.pm` (in
+  parts of its code we don't use)
+- `Locale::gettext`: only used to display translated error messages
+- Nicolas François "hopes" `DynaLoader` is safe, and has "no reason to
+  think that `Encode` is not safe"
+- Nicolas François has "no reason to think that `Encode::Guess` is not
+  safe". The po plugin nevertheless avoids using it by defining the
+  input charset (`file_in_charset`) before asking `Transtractor` to
+  read any file. NB: this hack depends on po4a internals to stay
+  the same.
+
+#### To be checked
+
+##### Locale::Po4a modules
+
+- the modules we want to use have to be checked, as not all are safe
+  (e.g. the LaTeX module's behaviour is changed by commands included
+  in the content); they may use regexps generated from the content; we
+  currently only use the `Text` module
+- the `Text` module does not run any external program
+- check that no module is loaded by `Chooser.pm`, when we tell it to
+  load the `Text` one
+- `nsgmls` is used by `Sgml.pm`
+
+##### Text::WrapI18N
+
+`Text::WrapI18N` can cause DoS (see the
+[Debian bug #470250](http://bugs.debian.org/470250)), but it is
+optional and we do not need the features it provides.
+
+It is loaded if available by `Locale::Po4a::Common`; looking at the
+code, I'm not sure we can prevent this at all, but maybe some symbol
+table manipulation tricks could work; overriding
+`Locale::Po4a::Common::wrapi18n` may be easier. I'm no expert at all
+in this field. Joey? [[--intrigeri]]
+
+##### Term::ReadKey
+
+`Term::ReadKey` is not a hard dependency in our case, *i.e.* po4a
+works nicely without it. But the po4a Debian package recommends
+`libterm-readkey-perl`, so it will probably be installed on most
+systems using the po plugin.
+
+If `$ENV{COLUMNS}` is not set, `Locale::Po4a::Common` uses
+`Term::ReadKey::GetTerminalSize()` to get the terminal size. How safe
+is this?
+
+Part of `Term::ReadKey` is written in C. Depending on the runtime
+platform, this function use ioctl, environment, or C library function
+calls, and may end up running the `resize` command (without
+arguments).
+
+IMHO, using Term::ReadKey has too far reaching implications for us to
+be able to guarantee anything wrt. security. Since it is anyway of no
+use in our case, I suggest we define `ENV{COLUMNS}` before loading
+`Locale::Po4a::Common`, just to be on the safe side. Joey?
+[[--intrigeri]]
+
+### msgmerge
+
+`refreshpofiles()` runs this external program. A po4a developer
+answered he does "not expect any security issues from it".
+
+### Fuzzing input
+
+I was not able to find any public information about gettext or po4a
+having been tested with a fuzzing program, such as `zzuf` or `fusil`.
+Moreover, some gettext parsers seem to be quite
+[easy to crash](http://fusil.hachoir.org/trac/browser/trunk/fuzzers/fusil-gettext),
+so it might be useful to bang msgmerge/po4a's heads against such
+a program in order to easily detect some of the most obvious DoS.
+[[--intrigeri]]
+
+> po4a was not fuzzy-tested, but according to one of its developers,
+> "it would be really appreciated". [[--intrigeri]]
+
 gettext/po4a rough corners
 --------------------------
 
@@ -246,8 +349,11 @@ gettext/po4a rough corners
   live in different directories): say bla.fr.po has been updated in
   repo2; pulling repo2 from repo1 seems to trigger a PO update, that
   changes bla.fr.po in repo1; then pushing repo1 to repo2 triggers
-  a PO update, that changes bla.fr.po in repo2; etc.; fixed in
-  `629968fc89bced6727981c0a1138072631751fee`?
+  a PO update, that changes bla.fr.po in repo2; etc.; quickly fixed in
+  `629968fc89bced6727981c0a1138072631751fee`, by disabling references
+  in Pot files. Using `Locale::Po4a::write_if_needed` might be
+  a cleaner solution. (warning: this function runs the external
+  `diff` program, have to check security)
 - new translations created in the web interface must get proper
   charset/encoding gettext metadata, else the next automatic PO update
   removes any non-ascii chars; possible solution: put such metadata
@@ -264,6 +370,13 @@ does. This is actually a duplicate for
 [[bugs/pagetitle_function_does_not_respect_meta_titles]], which might
 be fixed by something like [[todo/using_meta_titles_for_parentlinks]].
 
+### source files format
+
+Markdown is supported, great, but what about others? The set of file
+formats supported both in ikiwiki and po4a probably is greater than
+`{markdown}`. Warning: the po4a modules are the place where one can
+expect security issues.
+
 Translation quality assurance
 -----------------------------