Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#1942 closed Bug (invalid)

Connector fails to respond correctly on invalid parameter

Reported by: Michael Osipov Owned by:
Priority: Normal Milestone: FCKeditor 2.6
Component: Server : PHP Version: FCKeditor 2.6 Beta
Keywords: WorksForMe Cc:

Description (last modified by Frederico Caldeira Knabben)

Fire the PHP Connector with http://www.fckeditor.net/fckeditor/editor/filemanager/connectors/php/connector.php?Command=CreateFolder&Type=Image&NewFolderName=%5C

It's a backslash, the expected response should be a sanitized folder with the name "_" but the response is just the XML BOM, that's it.

Change History (10)

comment:1 Changed 11 years ago by Frederico Caldeira Knabben

Description: modified (diff)
Keywords: Confirmed added
Milestone: FCKeditor 2.6

comment:2 Changed 11 years ago by Michael Osipov

Summary: Connector fails respond correctly on invalid parameterConnector fails to respond correctly on invalid parameter

comment:3 Changed 11 years ago by Wiktor Walc

Keywords: WorksForMe added; Confirmed removed

Did it happen when using FCKeditor or did you launch that command manually? This command fails because CurrentFolder is missing in the query string.

Such situation shouldn't hapen when running FCKeditor and is secure enough to prevent against hackers (according to the rule that in such situations less information is better). If CurrentFolder is supplied, new folder with "_" name is created.

So the question is: should we care about standard responses when someone tries to hack the FCKeditor?

comment:4 Changed 11 years ago by Michael Osipov

I called it directly while reenginering the response.

As far as I am concerned, the connector should be fail-safe. With such a simple command I was able to disable the file browser for hours!

I implemented a strong parameter check in Java. check here the doGet method with the if else cascade.

comment:5 Changed 11 years ago by Wiktor Walc

Resolution: invalid
Status: newclosed

I don't see a problem here. If CurrentFolder is missing, further execution is stopped and an empty response is being sent. I think it is enough because it will happen only if someone manipulate manually with the file browser during hacking attempts. So in my opinion, this particular thing is handled correctly. Please reopen this ticket if I'm missing something important here.

You're absolutely right that the connector should be fail-safe and for example the control characters problem (#1945) that can break the XML response is something really needed (nice catch btw!).

comment:6 Changed 11 years ago by Michael Osipov

You stste yourself that the connector should be fail-safe this means that there has always to be a valid XML response deonting which error has occured leaving no room for invalid cracking!

comment:7 Changed 11 years ago by Frederico Caldeira Knabben

Priority: HighNormal

Ok... let me summarize two considerations about this:

  • "Valid requests" (like #1945) MUST always return valid XML responses, and not break the File Browser.
  • "Invalid requests" MAY return valid XML responses (with an error), but can also return nothing.

The fact is that the connector is made to run with the File Browser. The File Browser will NEVER send "invalid requests" to the connector (hopefully), so the File Browser MUST always receive valid XML.

At the other hand, it is possible to make manual calls to the connector, just like the reported case. Quite useful for hacking it. If it is an "invalid request", it is senseless doing any kind of job to properly reply XML. Actually, to minimize the request impact on the server, the response can be closed immediately once detected the invalidity, without further processing. Sending valid XML in those cases are optional, and it is up to the connector implementer.

Of course, for invalid requests, we are talking about well evident things, like the missing CurrentFolder parameter in the querystring. Invalid chars and so on must be handled properly, because there is a possibility that it could be sent by the File Browser.

When talking with Mike yesterday, I didn't paid attention to the missing CurrentFolder parameter. I thought it was a valid request with a strange NewFolderName value. Wiktor made it clear that it is invalid.

But the BOM reply is still something strange. I've just run our _dev/php_bom_finder.php script and no PHP files with BOM have been found. This is just a curiosity fact though.

For me we can close this ticket.

comment:8 in reply to:  7 ; Changed 11 years ago by Michael Osipov

Replying to fredck:

But the BOM reply is still something strange. I've just run our _dev/php_bom_finder.php script and no PHP files with BOM have been found. This is just a curiosity fact though.

maybe a new ticket is appropriate for?

comment:9 in reply to:  7 Changed 11 years ago by Michael Osipov

Replying to fredck:

Ok... let me summarize two considerations about this:

  • "Valid requests" (like #1945) MUST always return valid XML responses, and not break the File Browser.
  • "Invalid requests" MAY return valid XML responses (with an error), but can also return nothing.

The fact is that the connector is made to run with the File Browser. The File Browser will NEVER send "invalid requests" to the connector (hopefully), so the File Browser MUST always receive valid XML.

At the other hand, it is possible to make manual calls to the connector, just like the reported case. Quite useful for hacking it. If it is an "invalid request", it is senseless doing any kind of job to properly reply XML. Actually, to minimize the request impact on the server, the response can be closed immediately once detected the invalidity, without further processing. Sending valid XML in those cases are optional, and it is up to the connector implementer. .. For me we can close this ticket.

this should be held down in the Server Side Integration document for clarification reasons!

comment:10 in reply to:  8 Changed 11 years ago by Frederico Caldeira Knabben

Replying to mosipov:

Replying to fredck:

But the BOM reply is still something strange. I've just run our _dev/php_bom_finder.php script and no PHP files with BOM have been found. This is just a curiosity fact though.

maybe a new ticket is appropriate for?

Wiktor found out that our config file in the server has the BOM :) It is not a problem with our code base fortunately.

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy