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

fix potential null dereference in ext_authz #36268

Merged
merged 4 commits into from
Oct 15, 2024

Conversation

antoniovleonti
Copy link
Contributor

@antoniovleonti antoniovleonti commented Sep 20, 2024

Commit Message: fix potential null dereference in ext_authz
Additional Description:

Previously, if ext_authz had emit filter state stats set to true and another filter added filter state under the ext authz filter's name, it would result in a null dereference. The member logging_info_ would not be set in initiateCall after seeing there was already data there. Later, we would dereference logging_info_ to update the stats as if it were initialized already.

I've added a check for a null logging_info_ and added logging & a stat for when there's a filter state naming collision.

I also made some readability improvements to the ext_authz test.

Risk Level: low
Testing: unit tested
Docs Changes: none
Release Notes: none
Platform Specific Features: none

@antoniovleonti
Copy link
Contributor Author

those are real test failures, don't waste too much time looking at this yet.

@tyxia tyxia self-assigned this Sep 20, 2024
Signed-off-by: antoniovleonti <[email protected]>
Signed-off-by: antoniovleonti <[email protected]>
Copy link
Member

@tyxia tyxia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM modulo nits, Thanks

/wait

Copy link
Member

@tyxia tyxia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thanks!

@tyxia tyxia merged commit 9c75441 into envoyproxy:main Oct 15, 2024
20 checks passed
@antoniovleonti antoniovleonti deleted the statfuzz branch October 15, 2024 14:58
grnmeira pushed a commit to grnmeira/envoy that referenced this pull request Oct 17, 2024
Commit Message: fix potential null dereference in ext_authz
Additional Description:

Previously, if ext_authz had emit filter state stats set to true and
another filter added filter state under the ext authz filter's name, it
would result in a null dereference. The member logging_info_ would not
be set in initiateCall after seeing there was already data there. Later,
we would dereference logging_info_ to update the stats as if it were
initialized already.

I've added a check for a null logging_info_ and added logging & a stat
for when there's a filter state naming collision.

I also made some readability improvements to the ext_authz test.

Risk Level: low
Testing: unit tested
Docs Changes: none
Release Notes: none
Platform Specific Features: none

---------

Signed-off-by: antoniovleonti <[email protected]>
Signed-off-by: Gustavo <[email protected]>
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

Successfully merging this pull request may close these issues.

3 participants