telefonicaid / fiware-orion

Context Broker and CEF building block for context data management, providing NGSI interfaces.
https://fiware-orion.rtfd.io/
GNU Affero General Public License v3.0
212 stars 264 forks source link

Forbidden chars translation not working in NGSIv2 responses #2754

Open fgalan opened 7 years ago

fgalan commented 7 years ago

Working in https://github.com/telefonicaid/fiware-orion/issues/2752 I have discovered that the process to translate forbidden chars in responses is not correctly applied in NGSIv2.

Just compare (NGSIv1):

      "statusCode" : {
        "code" : "500",
        "reasonPhrase" : "Internal Server Error",
        "details" : "Sort operation used more than the maximum RAM. You should create an index. Check Orion documentation (Database administration section)."
      }

with (NGSIv2):

{
    "description": "Sort operation used more than the maximum RAM. You should create an index. Check Orion documentation (Database administration section).",
    "error": "InternalServerError"
}

(At the end I didn't use messages with ()

aarranz commented 7 years ago

What is the problem of returning parenthesis inside JSON strings? I think that returning parenthesis is a better option than using XML entities (( and )). What is the reason for encoding them using another standard when then can be used directly?

fgalan commented 7 years ago

In general (not only regarding (, but with any other of the forbidden chars) the reason is to avoid "dangerous" text derived from user input to appear in Orion responses.

By "dangerous" we mean text that, interpreted as code, could cause injection attacks when interpreted by the client receiving the responses from Orion. Thus, the forbidden chars are "key" syntactial chars used by common programming languages that could be potentially used in those attacks. Removing from any Orion interaction these "key" chars it is almost impossible to compose any text that interpreted as code could cause an injection attack.

From long time ago it has been a matter of discussion (and controversy :) if the current set composed of 8 characters is fine or it is too restricted. However, given that security is a very sensitive topic, we prefer to be conservative regarding this and to keep them as they are now (at least in the short term).

Side note: note that the XML encoding you suggest will not work, as ; is also part of the forbidden chars set. However, note that URL encoding will work.

aarranz commented 7 years ago

Sorry, but I still don't understand 😅. So, please illuminate me if I wrong. I think that the first error is to use data as code 😉 . None of the mainstream libraries used for parsing JSON will treat string data as executable code. What language/environment are you considering vulnerable?

From a point of view of a web developer, JSON.parse is available on all browsers from 2009 (there should be no one using eval for parsing JSON data). Also, support for CORS requests is fully operative from 2013, so JSONP (the other browser technique that may expose a problem in this regard) is not needed anymore. Moreover, JSONP is advertised as an insecure technology, e.g. this is what Wikipedia says:

Due to inherent insecurities, JSONP is being replaced by CORS.

NodeJS is not affected by this as its impossible to JSONP, and it got support for JSON.parse from the beginning. I don't see any potential problem using python, ruby and other interpreted languages and, evidently, I don't see any problem on compiled languages.

Side note: I didn't suggest any XML encoding, I have taken it from the example in your first post 😅

fgalan commented 7 years ago

Sorry, but I still don't understand

We have probably reached the point in which a skype talk in our common native language ;) could help to clarify.