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

add more tlvs for completeness #124

Closed
wants to merge 1 commit into from
Closed

Conversation

catenacyber
Copy link
Contributor

Found https://hackerone.com/bugs?subject=user&report_id=3022041

Also finds a dummy memory leak from API usage fixed by catenacyber/curl@7611370

@bagder bagder requested a review from cmeister2 March 6, 2025 12:52
#define TLV_TYPE_PROXY 53
#define TLV_TYPE_PROXYTYPE 54

#define TLV_TYPE_PROXYUSERPWD 100
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have a mild preference for these values to be block aligned to the right like the others, but I'm not going to be militant about it.

@cmeister2
Copy link
Collaborator

One minor comment but I think this is a good addition, once the main issue has been resolved.

@catenacyber
Copy link
Contributor Author

One minor comment but I think this is a good addition, once the main issue has been resolved.

What is the main issue ?
The leak in curl with CURLOPT_PROXYUSERNAME and CURLOPT_PROXYUSERPWD ?

@cmeister2
Copy link
Collaborator

One minor comment but I think this is a good addition, once the main issue has been resolved.

What is the main issue ? The leak in curl with CURLOPT_PROXYUSERNAME and CURLOPT_PROXYUSERPWD ?

Yes - we need the CI to be clean before this can get merged in. I'm happy if you want to split this into two MRs so we can get this into the fuzzing overnights.

@catenacyber
Copy link
Contributor Author

Ok, I created curl/curl#16599 I see curl/curl@c4cd0ae is already fixed in curl as well

@cmeister2
Copy link
Collaborator

Closed via #125 - I'll create a WIP MR with the backout for the TLV changes I made.

@cmeister2 cmeister2 closed this Mar 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants