From: Jacek Caban Subject: [PATCH 2/2] secur32: Don't change input buffer in InitializeSecurityContext. Message-Id: Date: Fri, 13 Jan 2017 19:57:48 +0100 Fixes bug 41218 by avoiding allocating input buffer as if it was output and reading from its uninitialized memory as it was part of handshake. Signed-off-by: Jacek Caban --- dlls/secur32/schannel.c | 13 ++++++++++--- dlls/secur32/tests/schannel.c | 31 +++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/dlls/secur32/schannel.c b/dlls/secur32/schannel.c index c6cc4d1..71f219c 100644 --- a/dlls/secur32/schannel.c +++ b/dlls/secur32/schannel.c @@ -732,7 +732,14 @@ schan_imp_session schan_session_for_transport(struct schan_transport* t) return t->ctx->session; } -static int schan_init_sec_ctx_get_next_buffer(const struct schan_transport *t, struct schan_buffers *s) +static int schan_init_sec_ctx_get_next_input_buffer(const struct schan_transport *t, struct schan_buffers *s) +{ + if (s->current_buffer_idx != -1) + return -1; + return schan_find_sec_buffer_idx(s->desc, 0, SECBUFFER_TOKEN); +} + +static int schan_init_sec_ctx_get_next_output_buffer(const struct schan_transport *t, struct schan_buffers *s) { if (s->current_buffer_idx == -1) { @@ -884,9 +891,9 @@ static SECURITY_STATUS SEC_ENTRY schan_InitializeSecurityContextW( ctx->req_ctx_attr = fContextReq; transport.ctx = ctx; - init_schan_buffers(&transport.in, pInput, schan_init_sec_ctx_get_next_buffer); + init_schan_buffers(&transport.in, pInput, schan_init_sec_ctx_get_next_input_buffer); transport.in.limit = expected_size; - init_schan_buffers(&transport.out, pOutput, schan_init_sec_ctx_get_next_buffer); + init_schan_buffers(&transport.out, pOutput, schan_init_sec_ctx_get_next_output_buffer); schan_imp_set_session_transport(ctx->session, &transport); /* Perform the TLS handshake */ diff --git a/dlls/secur32/tests/schannel.c b/dlls/secur32/tests/schannel.c index 4b8adc4..ead0e8c 100644 --- a/dlls/secur32/tests/schannel.c +++ b/dlls/secur32/tests/schannel.c @@ -636,6 +636,36 @@ static int receive_data(SOCKET sock, SecBuffer *buf) return received; } +static void test_InitializeSecurityContext(void) +{ + SCHANNEL_CRED cred; + CredHandle cred_handle; + CtxtHandle context; + SECURITY_STATUS status; + SecBuffer out_buffer = {1000, SECBUFFER_TOKEN, NULL}; + SecBuffer in_buffer = {0, SECBUFFER_EMPTY, NULL}; + SecBufferDesc out_buffers = {SECBUFFER_VERSION, 1, &out_buffer}; + SecBufferDesc in_buffers = {SECBUFFER_VERSION, 1, &in_buffer}; + ULONG attrs; + + init_cred(&cred); + cred.grbitEnabledProtocols = SP_PROT_TLS1_CLIENT; + cred.dwFlags = SCH_CRED_NO_DEFAULT_CREDS|SCH_CRED_MANUAL_CRED_VALIDATION; + status = AcquireCredentialsHandleA(NULL, (SEC_CHAR *)UNISP_NAME_A, SECPKG_CRED_OUTBOUND, NULL, + &cred, NULL, NULL, &cred_handle, NULL); + ok(status == SEC_E_OK, "AcquireCredentialsHandleA failed: %08x\n", status); + if (status != SEC_E_OK) return; + + status = InitializeSecurityContextA(&cred_handle, NULL, (SEC_CHAR *)"localhost", + ISC_REQ_CONFIDENTIALITY|ISC_REQ_STREAM|ISC_REQ_ALLOCATE_MEMORY, + 0, 0, &in_buffers, 0, &context, &out_buffers, &attrs, NULL); + ok(status == SEC_I_CONTINUE_NEEDED, "Expected SEC_I_CONTINUE_NEEDED, got %08x\n", status); + + FreeContextBuffer(out_buffer.pvBuffer); + DeleteSecurityContext(&context); + FreeCredentialsHandle(&cred_handle); +} + static void test_communication(void) { int ret; @@ -940,5 +970,6 @@ START_TEST(schannel) test_cread_attrs(); testAcquireSecurityContext(); + test_InitializeSecurityContext(); test_communication(); }