po: more security-related reports

[git.ikiwiki.info.git] / doc / plugins / po.mdwn

diff --git a/doc/plugins/po.mdwn b/doc/plugins/po.mdwn
index 544c7ef62b880c7dd72e572597cebba62039e783..4350e1a7311b36ae77943619afb186bd2750ede6 100644 (file)
--- a/doc/plugins/po.mdwn
+++ b/doc/plugins/po.mdwn
@@ -198,14 +198,18 @@ Also, when the plugin has just been enabled, or when a page has just
 been declared as being translatable, the needed POT and PO files are
 created, and the PO files are checked into version control.
 
 been declared as being translatable, the needed POT and PO files are
 created, and the PO files are checked into version control.
 

-Discussion pages
-----------------
+Discussion pages and other sub-pages
+------------------------------------

 
 Discussion should happen in the language in which the pages are
 written for real, *i.e.* the "master" one. If discussion pages are
 enabled, "slave" pages therefore link to the "master" page's
 discussion page.
 
 
 Discussion should happen in the language in which the pages are
 written for real, *i.e.* the "master" one. If discussion pages are
 enabled, "slave" pages therefore link to the "master" page's
 discussion page.
 

+Likewise, "slave" pages are not supposed to have sub-pages;
+[[WikiLinks|wikilink]] that appear on a "slave" page therefore link to
+the master page's sub-pages.
+

 Translating
 -----------
 
 Translating
 -----------
 


@@ -215,6 +219,22 @@ interface could also be implemented at some point).
 If [[tips/untrusted_git_push]] is setup, one can edit the PO files in one's
 preferred `$EDITOR`, without needing to be online.
 
 If [[tips/untrusted_git_push]] is setup, one can edit the PO files in one's
 preferred `$EDITOR`, without needing to be online.
 

+Markup languages support
+------------------------
+
+Markdown is well supported. Some other markup languages supported by
+ikiwiki mostly work, but some pieces of syntax are not rendered
+correctly on the slave pages:
+
+* [[reStructuredText|rst]]: anonymous hyperlinks and internal
+  cross-references
+* [[wikitext]]: conversion of newlines to paragraphs
+* [[creole]]: verbatim text is wrapped, tables are broken
+* [[html]] and LaTeX: not supported yet; the dedicated po4a modules
+  could be used to support them, but they would need a security audit
+* other markup languages have not been tested.
+
+

 TODO
 ====
 
 TODO
 ====
 


@@ -281,8 +301,6 @@ an initial goal, and analysing in detail the possible issues.
   read any file. NB: this hack depends on po4a internals to stay
   the same.
 
   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
 ##### Locale::Po4a modules
 
 The modules we want to use have to be checked, as not all are safe


@@ -299,24 +317,21 @@ means the `Text` module only.
   variables; they seem safe to me, but someone more expert than me
   will need to check. Joey?
 
   variables; they seem safe to me, but someone more expert than me
   will need to check. Joey?
 

+  > Freaky code, but seems ok due to use of `quotementa`.
+
+#### To be checked
+

 ##### 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.
 
 ##### 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]]
-
-> Update: Nicolas François suggests we add an option to po4a to
-> disable it. It would do the trick, but only for people running
-> a brand new po4a (probably too late for Lenny). Anyway, this option
-> would have to take effect in a `BEGIN` / `eval` that I'm not
-> familiar with. I can learn and do it, in case no Perl wizard
-> volunteers to provide the po4a patch. [[--intrigeri]]
+> I proposed a patch based on Joey's to po4a-devel, allowing to fully
+> disable this module's use. When it is merged upstream, we'll need to add
+> `use Locale::Po4a::Common qw(nowrapi18n)` to `po.pm`, before loading
+> any other `Locale::Po4a` module. A versioned dependency may be needed.
+> --[[intrigeri]]

 
 ##### Term::ReadKey
 
 
 ##### Term::ReadKey
 


@@ -325,42 +340,29 @@ 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.
 
 `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?
+`Term::ReadKey` has too far reaching implications for us to
+be able to guarantee anything wrt. security.

 
 

-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).
+> The option that disables `Text::WrapI18N` also disables
+> `Term::ReadKey` as a consequence. [[--intrigeri]]

 
 

-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

 
 

-> Update: adding an option to disable `Text::WrapI18N`, as Nicolas
-> François suggested, would as a bonus disable `Term::ReadKey`
-> as well. [[--intrigeri]]
+`refreshpofiles()` runs this external program.

 
 

-### msgmerge
+A po4a developer answered he does "not expect any security issues from
+it". I did not manage to crash it with `zzuf`, nor was able to find
+any past security holes.

 
 

-`refreshpofiles()` runs this external program. A po4a developer
-answered he does "not expect any security issues from it".
+### msgfmt

 
 

-### Fuzzing input
+`isvalidpo()` runs this external program.

 
 

-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]]
+* I could not manage to make it behave badly using zzuf, it exits
+  cleanly when too many errors are detected.
+* I could not find any past security holes.

 
 

-> po4a was not fuzzy-tested, but according to one of its developers,
-> "it would be really appreciated". [[--intrigeri]]
+### Fuzzing input

 
 Test conditions:
 
 
 Test conditions:
 


@@ -404,6 +406,9 @@ and
 Perl seems to exit cleanly, and an incomplete PO file is written on
 disk. I not sure whether if this is a bug in Perl or in `Po.pm`.
 
 Perl seems to exit cleanly, and an incomplete PO file is written on
 disk. I not sure whether if this is a bug in Perl or in `Po.pm`.
 

+> It's fairly standard perl behavior when fed malformed utf-8. As long as it doesn't
+> crash ikiwiki, it's probably acceptable. Ikiwiki can do some similar things itself when fed malformed utf-8 (doesn't crash tho) --[[Joey]]
+

 #### po4a-translate
 
 `po4a-translate` uses more or less the same po4a features as our
 #### po4a-translate
 
 `po4a-translate` uses more or less the same po4a features as our


@@ -432,18 +437,9 @@ While:
 
 ... seems to lose the fight, at the `readpo(LICENSES.fr.po)` step,
 against some kind of infinite loop, deadlock, or any similar beast.
 
 ... seems to lose the fight, at the `readpo(LICENSES.fr.po)` step,
 against some kind of infinite loop, deadlock, or any similar beast.

-It does not seem to eat memory, though.
-
-Whatever format module is used does not change anything. This is thus
-probably a bug in po4a's core or in a lib it depends on.
-
-The sub `read`, in `TransTractor.pm`, seems to be a good debugging
-starting point.

 
 

-#### msgmerge
-
-`msgmerge` is run in our `refreshpofiles` function. I did not manage
-to crash it with `zzuf`.
+The root of this bug lies in `Text::WrapI18N`, see above for
+possible solutions.

 
 gettext/po4a rough corners
 --------------------------
 
 gettext/po4a rough corners
 --------------------------


@@ -463,53 +459,51 @@ gettext/po4a rough corners
   into the Pot file, and let it propagate; should be fixed in
   `773de05a7a1ee68d2bed173367cf5e716884945a`, time will tell.
 
   into the Pot file, and let it propagate; should be fixed in
   `773de05a7a1ee68d2bed173367cf5e716884945a`, time will tell.
 

-Page titles in links
---------------------
-
-To use the page titles set with the [meta](plugins/meta) plugin when
-rendering links would be very much nicer, than the current
-"filename.LL" format. This is actually a duplicate for
-[[bugs/pagetitle_function_does_not_respect_meta_titles]].
-
-Page formats
+Better links

 ------------
 
 ------------
 

-Markdown is well supported, great, but what about others?
+### Page title in links

 
 

-The [po](plugins/po) uses `Locale::Po4a::Text` for every page format;
-this can be expected to work out of the box with most other wiki-like
-formats supported by ikiwiki. Some of their ad-hoc syntax might be
-parsed in a strange way, but the worst problems I can imagine would be
-wrapping issues; e.g. there is code in po4a dedicated to prevent
-re-wrapping the underlined Markdown headers.
+Using the fix to
+[[bugs/pagetitle_function_does_not_respect_meta_titles]] from
+[[intrigeri]]'s `meta` branch, the generated links' text is based on
+the page titles set with the [[meta|plugins/meta]] plugin. This has to
+be merged into ikiwiki upstream, though.

 
 

-While it would be easy to better support formats such as [[html]] or
-LaTeX, by using for each one the dedicated po4a module, this can be
-problematic from a security point of view.
+Robustness tests
+----------------

 
 

-**TODO**: test the more popular formats and write proper documentation
-about it.
+### Enabling/disabling the plugin

 
 

-Translation quality assurance
------------------------------
+- enabling the plugin with `po_translatable_pages` set
+- enabling the plugin without `po_translatable_pages` set: **OK**
+- disabling the plugin: **OK**

 
 

-Modifying a PO file via the CGI must be forbidden if the new version
-is not a valid PO file. As a bonus, check that it provides a more
-complete translation than the existing one.
+### Changing the plugin config

 
 

-A new `cansave` type of hook would be needed to implement this.
+- adding existing pages to `po_translatable_pages`: **OK**
+- removing existing pages from `po_translatable_pages`: **OK**
+- adding a language to `po_slave_languages`: **OK**
+- removing a language from `po_slave_languages`: **OK**
+- changing `po_master_language`: **OK**
+- replacing `po_master_language` with a language previously part of
+  `po_slave_languages`: needs two rebuilds, but **OK** (this is quite
+  a perverse test actually)

 
 

-Note: committing to the underlying repository is a way to bypass
-this check.
+### Creating/deleting/renaming pages

 
 

-Broken links
-------------
+All cases of master/slave page creation/deletion/rename, both via RCS
+and via CGI, have been tested.
+
+### Misc

 
 

-See [[contrib/po]].
+- general test with `usedirs` disabled: **OK**
+- general test with `indexpages` enabled
+- general test with `po_link_to=default`

 
 Documentation
 -------------
 
 Maybe write separate documentation depending on the people it targets:
 
 Documentation
 -------------
 
 Maybe write separate documentation depending on the people it targets:

-translators, wiki administrators, hackers. This plugin is maybe
-complex enough to deserve this.
+translators, wiki administrators, hackers. This plugin may be complex
+enough to deserve this.