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

GitLab webhook event validator vulnerable to timing attacks #2391

Closed
cedws opened this issue Jul 14, 2022 · 1 comment · Fixed by #2392
Closed

GitLab webhook event validator vulnerable to timing attacks #2391

cedws opened this issue Jul 14, 2022 · 1 comment · Fixed by #2392

Comments

@cedws
Copy link
Contributor

cedws commented Jul 14, 2022

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request. Searching for pre-existing feature requests helps us consolidate datapoints for identical requirements into a single place, thank you!
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment.

Overview of the Issue

I was just passing through the code and noticed that the GitLab webhook event validator code does not use a constant time comparison function to validate the webhook secret. In theory it would be possible to use a timing attack to recover this secret as an attacker and then forge webhook events. GitHub and Bitbucket event validator code is not vulnerable because they make use of HMACs as far as I can see.

The risk of this being exploited is probably fairly low so I think it's safe to report publicly like this.

// Validate secret if specified.
headerSecret := r.Header.Get(secretHeader)
secretStr := string(secret)
if len(secret) != 0 && headerSecret != secretStr {
return nil, fmt.Errorf("header %s=%s did not match expected secret", secretHeader, headerSecret)
}

I will follow up with a PR to fix this shortly.

@cedws cedws added the bug Something isn't working label Jul 14, 2022
@chenrui333 chenrui333 added security and removed bug Something isn't working labels Jul 15, 2022
@lkysow
Copy link
Member

lkysow commented Jul 15, 2022

Thanks @cedws! In the future just a heads up that we have a different process for security issues: https://github.com/runatlantis/atlantis/blob/master/CONTRIBUTING.md#reporting-security-issues

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

Successfully merging a pull request may close this issue.

4 participants