Extension talk:DatabaseFetchHook

From MediaWiki.org
Jump to navigation Jump to search

Hm... interesting, and scary, approach to security :) It is probably effective in hiding sensitive data, but may also lead to corruption (when mangled/truncated data is written back to the DB). Also I don't quite see how the hook knows what fields exist under which name in the object - it would have to know and analyze the queries select clause for that, I think.

All the security directives are in the text, so it only needs to check if the old_text column is in the requested row.
...unless there is something like x.old_text as somethingelse in the query. But i'm more worried about possible corruption. Although, if this deals only with full text, it's probably OK - the full text should only be changed when the user is editing a page. -- Duesentrieb 12:56, 5 March 2007 (UTC)

Anyway, there appears to be a largish loophole in this: in several places, fetchRow is used instead of fetchObject - notably, in Pager.php and Revision.php. This would have to be covered too. -- Duesentrieb 11:21, 5 March 2007 (UTC)

Yeah I saw fetchRow, but thought it wouldn't matter for now since the main holes to cover are the XML-export, search and feeds which all use fetchObject. I agree it's a bit of a scary way, so I've changed its status from beta to experimental ;-) --Nad 12:07, 5 March 2007 (UTC)
Actually, thinking about it... all access to full text goes through the Revision object, afaik (otherwise, code for accessing external storage and resolving stubbed text entries would have to be duplicated). Perhaps that would be a better place to hook in.
Another thing to worry about is parsercache, objectcache and querycache. Those may hold critical text, and not be checked when it is retrieved. -- Duesentrieb 12:56, 5 March 2007 (UTC)

I have tried to put the extension into MediaWiki 1.9.3 but I receive the following messages:

Warning: Missing argument 1 for LoadBalancer::LoadBalancer(), called in /usr/local/apache2/htdocs/wiki/extensions/DatabaseFetchObject.php on line 53 and defined in /usr/local/apache2/htdocs/wiki/includes/LoadBalancer.php on line 27
Warning: Invalid argument supplied for foreach() in /usr/local/apache2/htdocs/wiki/includes/LoadBalancer.php on line 44

-- 19:26, 18 May 2007 (UTC)

Should work now (haven't tested it though) but found that MW1.9 has made the $servers parameter in the constructor non-optional. Remember this is a highly scary and experimental hook! --Nad 23:00, 18 May 2007 (UTC)