Hi Cyril,
I'm fine with applying your series except a minor detail to fix here :
On Sun, Nov 30, 2014 at 11:51:09PM +0100, Cyril Bonté wrote:
> +static int external_check_add_env(struct check *check, const char *envname,
> const char *value, int *idx)
> +{
> + if (*idx % EXTCHECK_ENV_ROOM == 0) {
> + /* Allocate enough space to store new environment variables */
> + char **tmp = realloc(check->envp, (EXTCHECK_ENV_ROOM * ((*idx /
> EXTCHECK_ENV_ROOM) + 1) + 1) * sizeof(check->envp));
> + if (tmp == NULL) {
> + Alert("Failed to allocate memory for new environment
> variables. Aborting\n");
> + return 1;
> + } else {
> + check->envp = tmp;
> + }
> + }
> + /* Instead of sending NOT_USED, sending an empty value is preferable */
> + if (strcmp(value, "NOT_USED") == 0) {
> + value = "";
> + }
> + check->envp[*idx] = malloc(strlen(envname) + strlen(value) + 2);
> + if (!check->envp[*idx]) {
> + Alert("Failed to allocate memory for the environment variable
> '%s'. Aborting.\n", envname);
> + return 1;
> + }
> + sprintf(check->envp[*idx], "%s=%s", envname, value);
Please use snprintf() with the allocated length and check the result (ie
reject <=0 and >len). Some OSes like OpenBSD emit link-time warning when
sprintf() is used, and after some time I tend to find they're right. While
the initial use case for sprintf() is generally OK, people modify the
format later without realizing the impacts so better be safe from the start.
Thanks!
Willy
