Question

After upgrading from Cairo to newest TWiki 4.0.4-2 attaching files with provided usr comment does not work correctly. Newly put file is not commited into (ci) corresponding RCS file in pub directory. So there is no versioning there :(. This is similar or exactly the same as in AttachmentVersionHistoryNotUpdating (which I just noticed - after creation of this question), but I think I provided more details here and the solution should be different than just changing ci command in cfg file.

I discovered that the problem is caused by insecure (tainted) $comment parameter sent to TWiki::Store::RcsWrap::_ci method and after adding appropriate writeDebug I have the following error:

        | 16 Aug 2006 - 13:51 | Insecure dependency in exec while running with -T switch at /opt/twiki/lib/TWiki/Sandbox.pm line 319.
        TWiki::Sandbox::sysCommand('TWiki::Sandbox=HASH(0x9765d0)','/usr/bin/ci  -q -l -m\'%ESCAPEDCOMMENT(otherwise Comment plugin below destroys this)%\' -t-none -w\'%USERNAME%\' %...','USERNAME','WojciechSeliga','FILENAME','/opt/twiki/pub/Sandbox/TestTopic3/HFile_vhdl.pm','COMMENT','none') called at /opt/twiki/lib/TWiki/Store/RcsWrap.pm line 426
        TWiki::Store::RcsWrap::_ci('TWiki::Store::RcsWrap=HASH(0x1633580)','none','WojciechSeliga','undef') called at /opt/twiki/lib/TWiki/Store/RcsWrap.pm line 143
        TWiki::Store::RcsWrap::addRevisionFromStream('TWiki::Store::RcsWrap=HASH(0x1633580)','Fh=GLOB(0xacc7f0)','none','WojciechSeliga') called at /opt/twiki/lib/TWiki/Store.pm line 968
        TWiki::Store::__ANON__() called at /opt/twiki/lib/CPAN/lib///Error.pm line 379
        eval {...} called at /opt/twiki/lib/CPAN/lib///Error.pm line 371
        Error::subs::try('CODE(0x15f6680)','HASH(0x1614d70)') called at /opt/twiki/lib/TWiki/Store.pm line 974
        TWiki::Store::__ANON__() called at /opt/twiki/lib/CPAN/lib///Error.pm line 379
        eval {...} called at /opt/twiki/lib/CPAN/lib///Error.pm line 371
        Error::subs::try('CODE(0x16249f0)','HASH(0x160f360)') called at /opt/twiki/lib/TWiki/Store.pm line 1012 
TWiki::Store::saveAttachment('TWiki::Store=HASH(0x12be600)','Sandbox','TestTopic3','HFile_vhdl.pm','TWiki::User=HASH(0x15b5560)','HASH(0x15fa840)') called at /opt/twiki/lib/TWiki/UI/Upload.pm line 230
        TWiki::UI::Upload::__ANON__() called at /opt/twiki/lib/CPAN/lib///Error.pm line 379
        eval {...} called at /opt/twiki/lib/CPAN/lib///Error.pm line 371
        Error::subs::try('CODE(0x168f750)','HASH(0x80a520)') called at /opt/twiki/lib/TWiki/UI/Upload.pm line 247
        TWiki::UI::Upload::upload('TWiki=HASH(0x6233a0)') called at /opt/twiki/lib/TWiki/UI.pm line 109
        TWiki::UI::__ANON__() called at /opt/twiki/lib/CPAN/lib///Error.pm line 379
        eval {...} called at /opt/twiki/lib/CPAN/lib///Error.pm line 371
        Error::subs::try('CODE(0x169a5b0)','HASH(0x1698a00)') called at /opt/twiki/lib/TWiki/UI.pm line 158
        TWiki::UI::run('CODE(0x12ac7e0)') called

In the filesystem underneath i have:

• empty RCS ,v file (no revision inside)
• correct (non-RCS) file which I uploaded

When I provide no comment while attaching file (the new one or just by "Manage" and that attach), everything works perfectly, so RCS file is correctly created (and there is revision there).

I applied my hand-made dirty fix, adding in RcsWrap.pm:

...
sub _ci {
    my( $this, $comment, $user, $date ) = @_;
 
    $comment = 'none' unless $comment;
 
# added by me - to make comments working (otherwise we have security problem by exec())
    $comment =~ /(.*)/;
    $comment = $1; 

... 

and then it (versioning of attachments) works fine.

It is only thanx to $comment set to "none" while normal topic change, that topic versioning works as expected.

Another problem I get - even while attaching files without comment - is a little bit corrupted version history table in "manage" attachment screen (using pattern skin):

Please notice spurious single quotation marks around author name (destroying links) and comment. These quatation marks are also inside RCS history:

revision 1.2    locked by: wwwrun;
date: 2006/08/16 12:04:17;  author: 'WojciechSeliga';  state: Exp;  lines: +1 -0
'the final test - check if it works'
----------------------------

I wonder if have something wrong with file permissions or is there some bug in newest TWiki... My Apache is running user wwwrun (group www). permissions are:

-r-xr-xr-x   1 wwwrun www   1055 Aug 15 11:00 upload

and:

gkscoc01:/opt/twiki # ls -l lib/TWiki/Sto*
-r--r-----  1 wwwrun www  65319 Aug 16 12:10 lib/TWiki/Store.pm
 
lib/TWiki/Store:
total 85
drwxr-xr-x   2 wwwrun www    176 Aug 16 14:15 .
drwxr-xr-x  12 wwwrun www   1152 Aug 16 12:10 ..
-r--r-----   1 wwwrun www  26283 Jul  8 08:00 RcsFile.pm
-r--r-----   1 wwwrun www  21735 Aug 15 11:00 RcsLite.pm
-r--r-----   1 wwwrun www  15274 Aug 16 14:13 RcsWrap.pm

Part of my configuration (I cannot send you full config due to confidentiality issues): COMMENT were escaped with %ESCAPEDCOMMENT% to allow using comment plubin below (otherwise it gets confused and escaping with ! doesn't work).

$cfg{ReplaceIfEditedAgainWithin} = "1800"; # 3600
$cfg{RCS}{ciCmd} = "/usr/bin/ci  -q -l -m'!%ESCAPEDCOMMENT%' -t-none -w'%USERNAME%' %FILENAME%"; # /usr/bin/ci  -q -u -m'!%ESCAPEDCOMMENT%' -t-none -w'%USERNAME%' %FILENAME%
$cfg{RCS}{unlockCmd} = "/usr/bin/rcs  -q -u %FILENAME%"; # /usr/bin/rcs  -q -u -M %FILENAME%
$cfg{RCS}{ciDateCmd} = "/usr/bin/ci -l  -q -mnone -t-none -d'%DATE%' -w'%USERNAME%' %FILENAME%"; # /usr/bin/ci -u  -q -mnone -t-none -d'%DATE%' -w'%USERNAME%' %FILENAME%

I am looking forward to your help.

---

After taking a look at AttachmentVersionHistoryNotUpdating I found out that modifier flags can be use to untaint variables (sub _buildCommandLine of Storage.pm). Probably it will solve all my problems. I will check it out soon and let you know.

BTW: Add comment in the box below does not work for me...

Environment

-- WojciechSeliga - 17 Aug 2006

Answer

After reading AttachmentVersionHistoryNotUpdating I noticed that various flags can be (Sandbox.pm -> sub _buildCommandLine) to untaint variables. I will try this settings in a minute and let you know if they work fine and also problems with '' in version history is solved now. If it will solve all problems, then I would recommend adding information about such problem I encountered to somewhere to upgrade guide - as UpgradeTwiki script by default leaves wrong values of rcs related settings and this error may be very difficult to find for inexperienced admins/developers as TWiki does not even complain to the client (the error is silently consumed by backend).

-- WojciechSeliga - 17 Aug 2006

I checked settings advised at AttachmentVersionHistoryNotUpdating - everything works fine now. If this is intended (by design) that %U should be used in ciCmd (potential security issues with %U)- then please add (or let me know to add) appropriate information in TWikiUpgradeGuide. I belive many persons can have the same problem after automatic migration.

-- WojciechSeliga - 17 Aug 2006

Has anybody from TWiki developers taken a look at this question here? I would like to close this issue...

-- WojciechSeliga - 21 Aug 2006

Wojciech, sorry no-one has been paying attention to, it's holiday season.

As I understand it, you had changed the definition of ciCmd in TWiki.cfg in Cairo. When you upgraded, the upgrade script detected this change and accepted your Cairo definition of ciCmd. This of course did not have the |U% instruction (which simply untaints the relevant parameter without further checks), so you got a taint failure. Is that a fair summary?

This had never been reported before, probably because no-one has upgraded a system where any of the RCS commands have been changed for the site. It is certainly worth documenting in TWikiUpgradeGuide. Perhaps you could raise a bug in http://develop.twiki.org/~develop/cgi-bin/view/Bugs/WebHome (click on "Create New Item" in the menu, proposing some words to add to that topic?

-- CrawfordCurrie - 23 Aug 2006

Crawford, you almost hit the point with the only one difference: we actually did not touch ciCmd in Cairo (which we installed last winter). While installing Dakar, I just ran upgrade script and it left those settings (erroneous) which I included above. I will raise a bug as you proposed.

-- WojciechSeliga - 23 Aug 2006

Change status to: AskedQuestions AssignedQuestions AnsweredQuestions ClosedUnanswered