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

[Ernesto Baschny [cron IT]]

Hi,

btw +1 on testing and reading, if that wasn't clear enough :)

I would prefer of course my -v4, but I also have no major problem with
Ingo's -v3.

Cheers,
Ernesto

Ernesto Baschny [cron IT] schrieb am 27.09.2010 19:27:
> 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
>