ssl_lib: added pointer SSL_CONNECTION check to NULL before dereferencing it in ossl_ctrl_internal() #22470

nv-dmd · 2023-10-23T10:55:24Z

In ssl_lib.c added pointer SSL_CONNECTION check to NULL before dereferencing it in ossl_ctrl_internal()

…ing it in ossl_ctrl_internal()

nhorman · 2023-10-23T18:24:10Z

ssl/ssl_lib.c

@@ -2908,6 +2908,9 @@ long ossl_ctrl_internal(SSL *s, int cmd, long larg, void *parg, int no_quic)
    long l;
    SSL_CONNECTION *sc = SSL_CONNECTION_FROM_SSL(s);

+    if (sc == NULL)
+        return 0;
+


There are a few reasons SSL_CONNECTION_FROM_SSL might resolve to NULL. Clearly if s is NULL it will. But it will also return NULL if:
OPENSSL_NO_QUIC is not defined and the ssl type is neither SSL_TYPE_SSL_CONNECTION nor SSL_TYPE_QUIC_CONNECTION
or
if OPENSSL_NO_QUICK is defined and the ssl type is not SSL_TYPE_SSL_CONNECTION

Is it worth detecting that rasing an error based on those condtions? Otherwise users will get a 0 return code and no error when what otherwise appears to be a valid ssl object is passed in

I corrected the code so that pointer SSL is checked first. If it is equal to NULL, then returned 0, since pointer SSL_CONNECTION will also be NULL.
Next comes checking SSL_CONNECTION for NULL only by ssl type. Pointer SSL_CONNECTION needs to be checked, since it will be dereferenced next.

Please revert this to the original fix. In other cases we do not raise error for these failures and IMO there is no point doing that here anyway as these errors can happen only with severe application bugs.

Reverted to the original fix.

…referencing it in ossl_ctrl_internal()

changed since

…ing it in ossl_ctrl_internal()

openssl-machine · 2023-10-26T01:00:18Z

This pull request is ready to merge

hlandau · 2023-10-26T14:43:24Z

This is also applicable to 3.2.

hlandau · 2023-10-26T14:45:19Z

Merged to 3.2 and master. Thank you.

…ing it in ossl_ctrl_internal() Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from #22470)

…referencing it in ossl_ctrl_internal() Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from #22470)

…ing it in ossl_ctrl_internal() Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from #22470)

…ing it in ossl_ctrl_internal() Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from #22470) (cherry picked from commit 24844be)

…referencing it in ossl_ctrl_internal() Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from #22470) (cherry picked from commit b9788ce)

…referencing it in ossl_ctrl_internal() Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from #22470) (cherry picked from commit b419fcc)

…ing it in ossl_ctrl_internal() Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from #22470) (cherry picked from commit 8dc82c0)

…ing it in ossl_ctrl_internal() Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from openssl/openssl#22470) Signed-off-by: fly2x <fly2x@hitls.org>

…referencing it in ossl_ctrl_internal() Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from openssl/openssl#22470) Signed-off-by: fly2x <fly2x@hitls.org>

…ing it in ossl_ctrl_internal() Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from openssl/openssl#22470) Signed-off-by: fly2x <fly2x@hitls.org>

ssl_lib: added pointer SSL_CONNECTION check to NULL before dereferenc…

7b31529

…ing it in ossl_ctrl_internal()

t8m previously approved these changes Oct 23, 2023

View reviewed changes

nhorman reviewed Oct 23, 2023

View reviewed changes

atishkov added 2 commits October 24, 2023 12:30

ssl_lib: added pointer SSL and SSL_CONNECTION check to NULL before de…

b3c81da

…referencing it in ossl_ctrl_internal()

ssl_lib: added pointer SSL and SSL_CONNECTION check to NULL before de…

7e7c356

…referencing it in ossl_ctrl_internal()

t8m removed the approval: review pending This pull request needs review by a committer label Oct 24, 2023

github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Oct 24, 2023

ssl_lib: added pointer SSL_CONNECTION check to NULL before dereferenc…

f609abe

…ing it in ossl_ctrl_internal()

t8m approved these changes Oct 24, 2023

View reviewed changes

t8m added the approval: review pending This pull request needs review by a committer label Oct 24, 2023

github-actions bot removed the severity: fips change The pull request changes FIPS provider sources label Oct 24, 2023

paulidale approved these changes Oct 25, 2023

View reviewed changes

paulidale added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Oct 25, 2023

openssl-machine added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Oct 26, 2023

hlandau added the branch: 3.2 Merge to openssl-3.2 label Oct 26, 2023

hlandau closed this Oct 26, 2023

nv-dmd deleted the check_ssl_pointer_in_SSL_ctrl_funtion branch October 26, 2023 15:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ssl_lib: added pointer SSL_CONNECTION check to NULL before dereferencing it in ossl_ctrl_internal() #22470

ssl_lib: added pointer SSL_CONNECTION check to NULL before dereferencing it in ossl_ctrl_internal() #22470

nv-dmd commented Oct 23, 2023

nhorman Oct 23, 2023

nv-dmd Oct 24, 2023

t8m Oct 24, 2023

nv-dmd Oct 24, 2023

openssl-machine commented Oct 26, 2023

hlandau commented Oct 26, 2023

hlandau commented Oct 26, 2023

ssl_lib: added pointer SSL_CONNECTION check to NULL before dereferencing it in ossl_ctrl_internal() #22470

ssl_lib: added pointer SSL_CONNECTION check to NULL before dereferencing it in ossl_ctrl_internal() #22470

Conversation

nv-dmd commented Oct 23, 2023

nhorman Oct 23, 2023

Choose a reason for hiding this comment

nv-dmd Oct 24, 2023

Choose a reason for hiding this comment

t8m Oct 24, 2023

Choose a reason for hiding this comment

nv-dmd Oct 24, 2023

Choose a reason for hiding this comment

openssl-machine commented Oct 26, 2023

hlandau commented Oct 26, 2023

hlandau commented Oct 26, 2023