painterjd / stoplight

Stoplight -- An Input Validation Framework for Python
Apache License 2.0
15 stars 5 forks source link

Provide some standardized validators for common functionality #11

Open BenjamenMeyer opened 6 years ago

BenjamenMeyer commented 6 years ago

Adding stoplight to another project, and it seems it would be good to do one of the following:

  1. Have stoplight provide some built-in validators
  2. Provide an add-on library (f.e stoplight-rules) for some built-in validators

The primary purpose is to enable having some various pre-defined rules for well known types that people can just use instead of having to figure out how to do it. Some example types that should be covered as a starting point:

Some of these may require adding bounds (length, min/max, etc) as configuration/parameters to the provided rules.

NOTE: This is in comparison to other libraries like voluptuous which provide extensive Schema support; voluptuous relies on type-casting thus enabling the Python VM to be cracked via the isinstance and other type-casting methods.

TomFaulkner commented 6 years ago

What do you mean here? "enabling the Python VM to be cracked via the isinstance and other type-casting methods.".

BenjamenMeyer commented 6 years ago

@TomFaulkner if you pass a random value to something that simply casts it from one memory type to another without validating that the data should be that kind of data you have a problem.

Sometimes we can take advantage of these kinds of things for a good benefit, for example in the https://en.wikipedia.org/wiki/Fast_inverse_square_root optimization in Quake3 Arena.

However, those same kinds of things can be utilized on inputs from external sources to compromise a project - and that's exactly what isinstance does. It doesn't validate that given value is actually compatible with an int type, it just does the equivalent of:

try:
    int(value)
except TypeError:
   return False
else:
    return True

Instead, before running int(value) the content of value should be proven that it's valid data to be passed to int().

TomFaulkner commented 6 years ago

Do you have any documentation saying this is how isinstance works or documentation on how it could potentially be exploited? It appears, and it's name implies, to be checking where the object is in question is an instance of a particular object. It does not do the equivalent of what your example code is, if it did isinstance('5', int) would return True, however it returns False. (It does return True on 1 being a bool, but that is because of inheritance and not the issue being discussed.)

From the current documentation for Python 3:

Return true if the object argument is an instance of the classinfo argument, or of a (direct, indirect or virtual) subclass thereof. If object is not an object of the given type, the function always returns false. If classinfo is a tuple of type objects (or recursively, other such tuples), return true if object is an instance of any of the types. If classinfo is not a type or tuple of types and such tuples, a TypeError exception is raised.

source: https://docs.python.org/3/library/functions.html#isinstance

isinstance source: https://github.com/python/cpython/blob/master/Objects/abstract.c#L2368

BenjamenMeyer commented 6 years ago

@TomFaulkner I had looked through the source a while back related to several of those kinds of things for another project. I don't have the details off hand - I'll have to see if I can find the reference I had; it might not be isinstance in particular, but it did have to do with how Python did type-casting (and it was directly into the Python source IIRC).

In any case, it's not safe to ever toss non-validated data into a method like isinstance, int, etc.x

If I had an exploit, that'd be a CVE. I don't have such a thing; however, this is calling out helping folks write good validators using known proven methods that would prevent such exploits even if they do exist instead of leaving the applications open to them with a false sense of security about being protected from them.

TomFaulkner commented 6 years ago

I agree on the need for good validation and sanitation, however, I think isinstance is the first step in that. How could you even begin to work with an object if you don't know what kind of object it is?

On Fri, Apr 27, 2018, 12:07 AM Benjamen Meyer notifications@github.com wrote:

@TomFaulkner https://github.com/TomFaulkner I had looked through the source a while back related to several of those kinds of things for another project. I don't have the details off hand - I'll have to see if I can find the reference I had; it might not be isinstance in particular, but it did have to do with how Python did type-casting (and it was directly into the Python source IIRC).

In any case, it's not safe to ever toss non-validated data into a method like isinstance, int, etc.x

If I had an exploit, that'd be a CVE. I don't have such a thing; however, this is calling out helping folks write good validators using known proven methods that would prevent such exploits even if they do exist instead of leaving the applications open to them with a false sense of security about being protected from them.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/painterjd/stoplight/issues/11#issuecomment-384865420, or mute the thread https://github.com/notifications/unsubscribe-auth/AOQ4TOZbVmery4i8p7E9rXB8hWz61jGXks5tsqebgaJpZM4TF4hT .

BenjamenMeyer commented 6 years ago

@TomFaulkner when dealing with HTTP APIs - you can use textual validation of the incoming data in most instances, f.e via Regex and similar means.