handling of duplicate _STRING and _FILENAME options in GOption

[Jan Kneschke <jan kneschke de>]

Hi,

I run into a problem with the error-handling of g_option_context_parse() and trying to understand the of the "context->changes" list.

As far as I see the job of context->changes is to revert the changes applied to the arg_data's by the parsing to the original state in case of an error like specifying a unknown option.

In case that a _STRING and _FILENAME option is specified twice restoring the original value fails and the arg_data points to a already freed memory location:

==53227== Invalid free() / delete / delete[]
==53227==    at 0x10000FC7C: free (vg_replace_malloc.c:366)
==53227==    by 0x100000DC1: main (goption-cli.c:46)
==53227==  Address 0x1056dd870 is 0 bytes inside a block of size 7 free'd
==53227==    at 0x10000FC7C: free (vg_replace_malloc.c:366)
==53227==    by 0x10005A623: parse_arg (in /opt/local/lib/libglib-2.0.0.dylib)
==53227==    by 0x10005AA56: parse_long_option (in /opt/local/lib/libglib-2.0.0.dylib)
==53227==    by 0x10005B553: g_option_context_parse (in /opt/local/lib/libglib-2.0.0.dylib)
==53227==    by 0x100000D77: main (goption-cli.c:34)

The problem is in parse_arg() for G_OPTION_ARG_FILENAME that does:

  /* first value */
  data = g_strdup (value);
  change = get_change (context, G_OPTION_ARG_FILENAME, entry->arg_data);
  change->prev.str = *(gchar **)entry->arg_data;   /* sets the original value */
  g_free (change->allocated.str); /* noop */
  change->allocated.str = data;
  *(gchar **)entry->arg_data = data;

  /* second value */
  data = g_strdup (value);
  change = get_change (context, G_OPTION_ARG_FILENAME, entry->arg_data); /* returns the same "Change" */
  change->prev.str = *(gchar **)entry->arg_data; /* points to the 'data' of the previous round */
  g_free (change->allocated.str); /* BUG: free's the previous data, makes change->prev.str pointing to a invalid location */
  change->allocated.str = data;
  *(gchar **)entry->arg_data = data;

in free_changes_list() it does:

  g_free (change->allocated.str);
  *(gchar **)change->arg_data = change->prev.str; /* change->prev.str is already invalid */
  
G_OPTION_ARG_STRING has the same problem.

The test-case is attached.

Attachment: goption-cli.c
Description: Binary data