[TYPO3-core] RFC: #15812: Add backend maintenance for login news

[Ernesto Baschny [cron IT]]

Jigal van Hemert schrieb am 27.09.2010 17:10:
> Hi,
> 
> On 26-9-2010 16:45, Ingo Renner wrote:
>> looks good, I just tweaked it a bit and also removed the strip_tags()
>> call for the header. I initially put them in to prevent XSS, but now
>> that we allow HTML in content, there's no need for it anymore...
> 
> Now an admin can introduce XSS in the news messages. The header text is
> htmlspecialchars'ed, but the body text isn't. It can easily be processed
> by  RemoveXSS::process(), can't it?
> 
> I tried it with < script >alert('XSS');< /script > (without the spaces)
> in both header and text, but it triggered my Antivirus software in the
> temporary internet files.
> 
> Trying to delete the System News record resulted in a red flash message:
> 
>     1: Attempt to delete record without delete-permissions
> 
> The record is however deleted (deleted is set to 1). I tested it with a
> record with only a single word in both header and text, but the same
> result here when deleting the record.
> 
> Overall I like the fact that this feature can be used more easily,
> however the problems I encountered need fixing IMO.

I thought about that too.

First reaction was like Steffen: "Why? An admin can do everything
anyway". But overall, most issues arise from the fact that the admin can
edit all content and he can *install extensions*. As soon as you disable
EM (which is now possible, as EM is a sysext), your admin cannot do
"everything".

Of course the backend is by far not safe for XSS from admin-users, but I
think we can at least disallow it to happen with new things we are
introducing.

So I agree with Jigal that on new features we should care to allow the
admin to do no more than what the new feature is supposed to to. Adding
a RemoveXSS call is probably the easiest way to handle it in this
situation, as we clearly mark that content from being "sanitizeable".

Now to the improved patch (first one created with PhpStorm, yea!):

1) Ingo mentioned "I think the feature belongs to the management view of
TYPO3 which is EXT:cms.". I disagree, as the login screen hardly has
anything to do with the "cms" extension. I would rather think this table
and its placement are a candidate for stddb/tables*, like the other
stuff that we have from start (sys_log, sys_languages, be_users, i.e.
stuff that we create on the root page)
=> so I moved the table definitions to that stddb\* places and the label
to lang/locallang_tca.xml.

2) I noticed the bug of the flash message after delete too. But this
doesn't seem to be related to *this* patch, as this also happens on
be_users for example.
=> So a further RFC should handle that fix.

3) I noticed that setting "starttime" *with the checkbox* to "today",
somehow that also sets the *time* fraction of the underlying integer to
"now", so the news doesn't appear today.
=> Also not related to this patch, another RFC required to fix.

4) Added the t3lib_div::removeXSS call when rendering the entry in the
login form.


My only issue with this was to get the icon working (I gave up after
some tries): I copied the one Ingo supplied to
"typo3/sysext/t3skin/images/icons/mimetypes/", cleared all cache, but it
won't render. Do I need to recreate the sprites.css in order for it to
work? We need some kind of manual for core developers for adding new
icons (maybe there is some already) :)


Sidenote to Ingo: I think it is important to discuss and bring in every
argument about a specific new feature in. Your reply to Benni was very
fruitful and made us aware of how you reached your decisions. Having
that "on list" will make it easy to judge the decisions "years later"
instead of thinking "maybe they didn't consider this or that aspect".

So you shouldn't be demotivated to bring new stuff to the core because
of that. We all love new features, and such discussions are part of a
normal team work.

Cheers,
Ernesto
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 15812-v4.patch
URL: <http://lists.typo3.org/pipermail/typo3-team-core/attachments/20100927/6ea0962c/attachment.asc>