ruby / ruby/openssl

Unchecked *_set_* calls

Open
#1,038 1 comment 0 reactions 0 assignees View on GitHub

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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.