Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DefaultFieldResolver: callback through ObjectAccess vulnerability #35

Open
simstern opened this issue Feb 15, 2021 · 4 comments
Open

DefaultFieldResolver: callback through ObjectAccess vulnerability #35

simstern opened this issue Feb 15, 2021 · 4 comments

Comments

@simstern
Copy link
Contributor

When using the DefaultFieldResolver there is the following risk:

Resolving an object through the "magic" ObjectAccess returning a property value that is callable, the resolver will call this function.

        if (is_object($source) && ObjectAccess::isPropertyGettable($source, $fieldName)) {
            $resolvedProperty = ObjectAccess::getProperty($source, $fieldName);
        }

        if (is_callable($resolvedProperty)) {
            return $resolvedProperty($source, $args, $context, $info);
        }

I noticed this when working with a user with firstName "Max". I do not have a specific resolver for User.

So, first the DefaultFieldResolver gets the firstName property from the User object through ObjectAccess::getProperty and assignes $resolvedProperty = 'Max'. Since Max is callable, this is executed.

This is quite risky when working with user input. Possibly only support Closures here?

@Sebobo
Copy link
Contributor

Sebobo commented Oct 14, 2021

@johannessteu do you know how to solve this in the best & secure way?

@DrillSergeant
Copy link

I came across this today as well, in my case a company with name "Tan" caused the Resolver to try to call tan().

@simstern
Copy link
Contributor Author

Just sumitted this PR: #38
It works for me for quite some time now.

@kdambekalns
Copy link
Contributor

This is indeed risky and probably (for most people) unexpected. Limiting the call to Closure values as in the suggested PR sounds reasonable to me… does that match the intention of the DefaultFieldResolver behaviour, @johannessteu?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants