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

State value in callback query doesn't compared with session stored value #218

Open
AlexanderSysoev opened this issue Jul 21, 2017 · 6 comments

Comments

@AlexanderSysoev
Copy link

AlexanderSysoev commented Jul 21, 2017

At BaseOAuth20Provider.cs:

        // Start with the Cross Site Request Forgery check.
        var callbackState = queryStringParameters[StateKey];
        if (string.IsNullOrEmpty(callbackState))
        {
            var errorMessage =
                "The callback querystring doesn't include a state key/value parameter. We need one of these so we can to a CSRF check. Please check why the request url from the provider is missing the parameter: " +
                StateKey + ". eg. &state=something...";
            TraceSource.TraceError(errorMessage);
            throw new AuthenticationException(errorMessage);
        }

May be this line of code needed:
callbackState.Equals(state, StringComparison.OrdinalIgnoreCase)

@AlexanderSysoev AlexanderSysoev changed the title State value in callback query string doesn't compared with session stored value State value in callback query doesn't compared with session stored value Jul 21, 2017
@PureKrome
Copy link
Member

Hi @AlexanderSysoev - thanks for filing this issue.

can you please give some sample data where this fails the check, so I can understand the problem more.

plz?

@AlexanderSysoev
Copy link
Author

AlexanderSysoev commented Jul 21, 2017

@PureKrome I've not tested yet, but I didn't find any comparison inside this region:
Cross Site Request Forgery checks -> state == state?

@PureKrome
Copy link
Member

OK - so what you're saying is that I'm missing the state check in this part of the code?

(still trying to grok the problem)

@AlexanderSysoev
Copy link
Author

OK - so what you're saying is that I'm missing the state check in this part of the code?

Exactly

@PureKrome
Copy link
Member

Ok - ta. gotcha! Phew :)

BTW - how did you figure this out?

@AlexanderSysoev
Copy link
Author

@PureKrome sorry for my bad communication skills)

how did you figure this out?

Just viewing the code)

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

2 participants