secur32: Getting real functionality into the NTLM provider
Alexandre Julliard
julliard at winehq.org
Tue Aug 23 13:39:52 CDT 2005
Kai Blin <blin at gmx.net> writes:
> + unsigned char buffer[SECUR32_MAX_BUF_LEN+6000];
> + BYTE bin[SECUR32_MAX_BUF_LEN];
You shouldn't allocate fixed size buffers on the stack, especially not
huge ones like that. You should compute the necessary size and
allocate a properly sized buffer from the heap.
> + SEC_CHAR client_user_arg[512];
> + SEC_CHAR client_domain_arg[512];
> + SEC_CHAR passwd[512] = {0};
Same here, there's nothing that guarantees the buffers are the right
size; using snprintf may avoid overflows but it doesn't make the code
behave correctly.
> +SECURITY_STATUS fork_helperA(PNegoHelperA *new_helper, const char *prog,
> + char * const argv[], char* passwd)
> +{
You should have a single fork helper that takes Unicode, you can't
pass ASCII chars to the Unix app anyway, you have to use
CP_UNIXCP. And more generally, you should do everything in Unicode.
> + if( ( pipe(pipe_in) < 0 ) || ( pipe(pipe_out) < 0 ) )
> + {
> + return SEC_E_INTERNAL_ERROR;
> + }
> +
> + helper->helper_pid = fork();
> +
> + helper->password = passwd;
> +
> + if(helper->helper_pid == -1)
> + {
> + return SEC_E_INTERNAL_ERROR;
> + }
You are leaking file descriptors in the error cases.
> + if(helper->helper_pid == 0)
> + {
> + /* We're in the child now */
> + close(0);
> + close(1);
> +
> + dup2(pipe_out[0], 0);
> + close(pipe_out[0]);
> + close(pipe_out[1]);
> +
> + dup2(pipe_in[1], 1);
> + close(pipe_in[0]);
> + close(pipe_in[1]);
> +
> + execvp(prog, argv);
> +
> + /* Whoops, we shouldn't get here. Big badaboom.*/
> + return SEC_E_INTERNAL_ERROR;
You shouldn't return from the child, you should pass the error back to
the parent and then exit().
> + {
> + helper->pipe_in = fdopen(pipe_in[0], "r");
> + close(pipe_in[1]);
> + helper->pipe_out = fdopen(pipe_out[1], "w");
> + close(pipe_out[0]);
> + }
You should avoid using stdio.
--
Alexandre Julliard
julliard at winehq.org
More information about the wine-devel
mailing list