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

std.ip: Always provide some form of fallback #3746

Merged
merged 1 commit into from
Nov 29, 2021

Conversation

dridi
Copy link
Member

@dridi dridi commented Nov 26, 2021

Otherwise valid code can panic on workspace exhaustion:

std.ip(req.http.X-Real-IP, std.ip(req.http.X-Client-IP, client.ip))

If the nested std.ip() call runs out of workspace, it will return a null
ip instead of the fallback, and since the outer std.ip() call is getting
a fallback argument, it will panic upon checking the suckaddr sanity.

Refs a3b26d0

@bsdphk
Copy link
Contributor

bsdphk commented Nov 29, 2021

I think this should be a VRT_fail() ?

@dridi
Copy link
Member Author

dridi commented Nov 29, 2021

We could.

In the scenario I got where the nested std.ip() call runs out of workspace, the nested call VRT_fails but the parent call panics upon getting a null IP.

Otherwise valid code can panic on workspace exhaustion:

    std.ip(req.http.X-Real-IP, std.ip(req.http.X-Client-IP, client.ip))

If the nested std.ip() call runs out of workspace, it will return a null
ip instead of the fallback, and since the outer std.ip() call is getting
a fallback argument, it will panic upon checking the suckaddr sanity.

Refs a3b26d0
@dridi dridi merged commit 3dd4d72 into varnishcache:master Nov 29, 2021
@dridi dridi deleted the std_ip_null_fallback branch November 29, 2021 14:47
@dridi
Copy link
Member Author

dridi commented Nov 29, 2021

I will submit a patch for the 6.0 branch too.

dridi added a commit to dridi/varnish-cache that referenced this pull request Nov 29, 2021
Otherwise valid code can panic on workspace exhaustion:

    std.ip(req.http.X-Real-IP, std.ip(req.http.X-Client-IP, client.ip))

If the nested std.ip() call runs out of workspace, it will return a null
ip instead of the fallback, and the outer std.ip() call will panic upon
checking the suckaddr sanity.

Refs varnishcache#3746
@dridi
Copy link
Member Author

dridi commented Nov 29, 2021

#3748

dridi added a commit that referenced this pull request Nov 30, 2021
Otherwise valid code can panic on workspace exhaustion:

    std.ip(req.http.X-Real-IP, std.ip(req.http.X-Client-IP, client.ip))

If the nested std.ip() call runs out of workspace, it will return a null
ip instead of the fallback, and the outer std.ip() call will panic upon
checking the suckaddr sanity.

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

Successfully merging this pull request may close these issues.

3 participants