Refactoring Proposal: TWiki Codebase Security Audit

Motivation

Reasons why this refactoring is needed, and why it is suitable for adding to the next release of TWiki.

Description

To fix the inherent insecurities, we will need to:

1. examine all shell calls, replace them with a common strategy that can be used by future authors (core & plugins)
   • ie, not just fix the current issue
   • this needs to work reliably and securely in all environments, e.g. also with a crashed system call issued in a mod_perl environments
2. clean up the un-taint code
3. validate all CGI and user paramters
4. what else?

In the long term, we need a better alerting mechanism to inform TWiki admins of problems, and ideally some system to automatically apply security hot fixes.

• NOTE: this will need to include some kind of TWikiApplication installation system, as one of the earlier TWikiSecurityAlerts required topic changes.

This work is intended to be done for Dakar, and as such will begin life in DEVELOP

This topic is the entry point to specific security measures for the Dakar codebase and for alerting administrators:

• SingleEntryPointForSystemCalls: One secure global function to be used by core code and Plugin code to issue system calls
• UntaintCodeDakarSecurityAudit: Audit and cleanup untaint code, including validation of CGI and user paramters
• TWikiSecurityAlertProcess: Process and mechanism of informing TWiki administrators of security alerts

Contributors:
-- SvenDowideit - 14 Nov 2004
-- PeterThoeny - 14 Nov 2004

Impact and Available Solutions

Note: Patch is attached as https://twiki.org/p/pub/Codev/TWikiCodebaseSecurityAudit/twiki-foo-bar-patch.diff. The patch is against the TWikiAlphaRelease of 15 Feb 2004.

Documentation

If necessary, developer documentation of new features and changed APIs introduced by this proposal.

Examples

Example uses of new features and changed APIs introduced by proposal.

Implementation

Any comments on how the refactoring is implemented or could be improved

Discussion

You should decide very early if you want to keep Perl 5.6.1 or even 5.005 compatibility. Perl 5.8 introduced a helpful "safe pipe to subprocess" command which should be used in the RCS interface.

-- FlorianWeimer

I don't think we can mandate 5.8 yet - too many web hosts, and older intranet servers, are still on 5.005 or 5.6. Not sure we really need that feature to be secure, though it might make life easier.

We do need to move to a filtering in approach (see Google:twiki+filtering+secure) - originally, TWiki went for filtering out, to enable I18N by default, but now that we have InternationalisationEnhancements there is less of a requirement for that.

Any Unicode support in future needs to be careful in filtering out overlong UTF-8 strings that provide alternative ways of encoding characters such as backquotes - the current UTF-8 URL code already does this.

-- RichardDonkin - 15 Nov 2004

One Man's Opinion:

The problem TWiki is having is very akin to the problem Perl is having, and I believe it's a result of acute Microsoftitis: "We must not break compatibility."

I agree that functional compatibility can't be broken, but that does not mean that the entire core doesn't need to be re-thought and re-worked with a security as a primary goal. Even if that means an upgrade to the platform requirment.

Actually, the Perl solution might be the best solution. Divide the tree into a supported legacy branch, (ie, the tree as it currently exists) and an complete from-the-ground rewrite branch. TWiki->NG would have the same mission as TWiki, but slightly different design goals and a much more stringent set of tests than can be applied to TWiki. It would also have a much more standard OSS/FS methodology for communication with the external community.

So, 5.8 or .10 can be mandated, just not for TWiki...

-- KevinKinnell - 22 Dec 2004

thanks Kevin that sounds about right to me, a non coder, and largely inline with the current svn development model anyway.

-- MattWilkie - 22 Dec 2004

Kevin - I agree completely, and you will find the rules a slightly different for the DevelopBranch - where the major focus revolves around maintainability and testability. if there are no tests for it, then the feature can be considered to be removeable. We are heading for haveing a defined functionality set, with tests, and from there it can be possible to re-write with confidence. If however you really want to re-write from scratch, you could easily do this work in the SCRATCH area

-- SvenDowideit - 27 Dec 2004

I believe we have nailed all untaints, and all but one system calls go though the sandbox; it is a contained problem. So I'm stting this to done.