You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
test_proto_authorization_request() segfault due to uninitialized value
Many thanks to Florian Weimer fweimer@redhat.com for his assistence
in helping diagnose the problem.
In test_proto_authorization_request() it creates a provider object but
fails to fully initialize it. In particular it fails to initialize
auth_request_method to OIDC_AUTH_REQUEST_METHOD_GET.
The function oidc_proto_authorization_request() in the file
src/proto.c has a weak test for provider->auth_request_method on line
646.
if (provider->auth_request_method == OIDC_AUTH_REQUEST_METHOD_POST) {
/* construct a HTML POST auto-submit page with the authorization request parameters /
} else {
/ construct the full authorization request URL */
}
The assumption is provider->auth_request_method must be one of
OIDC_AUTH_REQUEST_METHOD_GET or OIDC_AUTH_REQUEST_METHOD_POST but if
the provider struct is not properly initialized it could be any random
value. Therefore the fact provider->auth_request_method does not equal
OIDC_AUTH_REQUEST_METHOD_POST does not imply it's
OIDC_AUTH_REQUEST_METHOD_GET. The test would also be a problem if
OIDC_AUTH_REQUEST_METHOD ever added a new enumerated value.
The defined values for OIDC_AUTH_REQUEST_METHOD are:
So what the test on line src/proto.c:646 is really saying is this:
if provider->auth_request_method != 1 then use the GET method.
The unit test works most of the time because it's unlikely the
unitialized auth_request_method member will be exactly 1. Except on
some architectures it is (e.g. s390x). Of course what's on the stack
is influenced by a variety of factors (all unknown).
The segfault occurs because oidc_proto_authorization_request() takes
the OIDC_AUTH_REQUEST_METHOD_POST branch and calls
oidc_proto_html_post() which then operates on more uninitialized
data. Specfically request->connection->bucket_alloc is
NULL. Fortunately the request object was intialized to zero via
apr_pcalloc() so at least bucket_alloc will consistetnly be NULL.
Here is the stack trace:
Program received signal SIGSEGV, Segmentation fault.
0x00007ffff6b9f67a in apr_bucket_alloc () from /lib64/libaprutil-1.so.0
(gdb) bt
from /lib64/libaprutil-1.so.0
data=0x6adfe0 "<!DOCTYPE HTML PUBLIC \"-//W3C//DTD HTML 4.01//EN\" \"http://www.w3.org/TR/html4/strict.dtd\">\n<html>\n <head>\n <meta http-equiv=\"Content-Type\" content=\"text/html; charset=utf-8\">\n <title>Submitting"...,
data_len=825, content_type=0x47311a "text/html", success_rvalue=-2)
at src/util.c:1307
title=0x46bf28 "Submitting...", html_head=0x0,
on_load=0x46bf0d "document.forms[0].submit()",
html_body=0x6add40 " <p>Submitting Authentication Request...</p>\n <form method=\"post\" action=\"https://idp.example.com/authorize\">\n <p>\n <input type=\"hidden\" name=\"response_type\" value=\"code\">\n <input"..., status_code=-2) at src/util.c:1349
url=0x465758 "https://idp.example.com/authorize", params=0x6acf30)
at src/proto.c:544
provider=0x7fffffffd260, login_hint=0x0,
redirect_uri=0x465790 "https://www.example.com/protected/",
state=0x4657b3 "12345", proto_state=0x68e5f0, id_token_hint=0x0,
nge=0x0, auth_request_params=0x0, path_scope=0x0) at src/proto.c:650
This patch does the following:
Initializes the provider struct created on the stack in
test_proto_authorization_request to zero so it's at least
initialized to known consistent values.
Initializes provider.auth_request_method to
OIDC_AUTH_REQUEST_METHOD_GET.
Initializes request->connection->bucket_alloc via
apr_bucket_alloc_create() so if one does enter that code it won't
segfault.
It's easy to verify the above observations.
If you explicitly intialize provider.auth_request_method to
OIDC_AUTH_REQUEST_METHOD_POST in test_proto_authorization_request()
instead of allowing it to be a random stack value you will segfault
every time. However if you initialize request->connection->bucket_alloc
the segfault goes away and the unit test correctly reports the error,
e.g.
Failed: # test_proto_authorization_request: error in oidc_proto_authorization_request (1): result "0" != expected "1"
WARNING: This patch does not address the test in proto.c:646. That
test should be augmented to assure only valid enumerated values are
operated on and if the enumerated value is not valid it should return
an error.
The text was updated successfully, but these errors were encountered:
test_proto_authorization_request() segfault due to uninitialized value
Many thanks to Florian Weimer fweimer@redhat.com for his assistence
in helping diagnose the problem.
In test_proto_authorization_request() it creates a provider object but
fails to fully initialize it. In particular it fails to initialize
auth_request_method to OIDC_AUTH_REQUEST_METHOD_GET.
The function oidc_proto_authorization_request() in the file
src/proto.c has a weak test for provider->auth_request_method on line
646.
if (provider->auth_request_method == OIDC_AUTH_REQUEST_METHOD_POST) {
/* construct a HTML POST auto-submit page with the authorization request parameters /
} else {
/ construct the full authorization request URL */
}
The assumption is provider->auth_request_method must be one of
OIDC_AUTH_REQUEST_METHOD_GET or OIDC_AUTH_REQUEST_METHOD_POST but if
the provider struct is not properly initialized it could be any random
value. Therefore the fact provider->auth_request_method does not equal
OIDC_AUTH_REQUEST_METHOD_POST does not imply it's
OIDC_AUTH_REQUEST_METHOD_GET. The test would also be a problem if
OIDC_AUTH_REQUEST_METHOD ever added a new enumerated value.
The defined values for OIDC_AUTH_REQUEST_METHOD are:
So what the test on line src/proto.c:646 is really saying is this:
if provider->auth_request_method != 1 then use the GET method.
The unit test works most of the time because it's unlikely the
unitialized auth_request_method member will be exactly 1. Except on
some architectures it is (e.g. s390x). Of course what's on the stack
is influenced by a variety of factors (all unknown).
The segfault occurs because oidc_proto_authorization_request() takes
the OIDC_AUTH_REQUEST_METHOD_POST branch and calls
oidc_proto_html_post() which then operates on more uninitialized
data. Specfically request->connection->bucket_alloc is
NULL. Fortunately the request object was intialized to zero via
apr_pcalloc() so at least bucket_alloc will consistetnly be NULL.
Here is the stack trace:
This patch does the following:
Initializes the provider struct created on the stack in
test_proto_authorization_request to zero so it's at least
initialized to known consistent values.
Initializes provider.auth_request_method to
OIDC_AUTH_REQUEST_METHOD_GET.
Initializes request->connection->bucket_alloc via
apr_bucket_alloc_create() so if one does enter that code it won't
segfault.
It's easy to verify the above observations.
If you explicitly intialize provider.auth_request_method to
OIDC_AUTH_REQUEST_METHOD_POST in test_proto_authorization_request()
instead of allowing it to be a random stack value you will segfault
every time. However if you initialize request->connection->bucket_alloc
the segfault goes away and the unit test correctly reports the error,
e.g.
Failed: # test_proto_authorization_request: error in oidc_proto_authorization_request (1): result "0" != expected "1"
WARNING: This patch does not address the test in proto.c:646. That
test should be augmented to assure only valid enumerated values are
operated on and if the enumerated value is not valid it should return
an error.
The text was updated successfully, but these errors were encountered: