Unchecked *_set_* calls
Nobody has claimed this yet.
- Dominant language
- C
- Stars
- 276
- Forks
- 200
- Avg merge
- 1d 19h
- Merged PRs (30d)
- 7
Description
There's a few calls to setter functions that are unchecked.
- X509_STORE_set_trust (can fail with invalid argument)
- X509_STORE_set_flags (can't fail with the current implementation, but docs don't confirm this)
- X509_STORE_set_purpose (can fail with invalid argument)
- SSL_SESSION_set_timeout (can fail with negative argument)
- TS_RESP_CTX_set_signer_key (can't fail with the current implementation, but docs don't confirm this, depends on failure of up_ref)
- Probably some more
Yet the setters on Ruby's side don't check the return value of these OpenSSL calls. That means that the user is not informed that their setter has failed. It also returns the new value even though the new value may not have been set.
It may be desirable to raise an exception. On the other hand it may break some code (although you could argue that the code was already wrong to begin with).
Could you please share your opinion on this? If wanted, I can send a PR.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Review the Ruby-side setters that call X509_STORE_set_trust, X509_STORE_set_flags, X509_STORE_set_purpose, SSL_SESSION_set_timeout, and TS_RESP_CTX_set_signer_key. Check how each OpenSSL return value is handled and determine the intended behavior when a setter fails. Done should include a decided error-handling policy and coverage for the affected calls.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c, ruby
- Domain
- cryptography, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 35/100