After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 751922 - SIGABRT from gtestutils.c:2356 on a fuzzed xls file
SIGABRT from gtestutils.c:2356 on a fuzzed xls file
Status: RESOLVED FIXED
Product: Gnumeric
Classification: Applications
Component: import/export MS Excel (tm)
git master
Other Linux
: Normal critical
: ---
Assigned To: Jody Goldberg
Jody Goldberg
http://jutaky.com/fuzzing/gnumeric_ca...
: 749907 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2015-07-03 19:37 UTC by jutaky
Modified: 2015-07-17 13:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Description jutaky 2015-07-03 19:37:26 UTC 
Git versions of glib, goffice, gnumeric, libgsf and libxml2.

Test case: http://jutaky.com/fuzzing/gnumeric_case_005-gtestutils.c.2356.xls

$ ssconvert gnumeric_case_005-gtestutils.c.2356.xls /tmp/out.gnumeric

Program received signal SIGABRT, Aborted.
0x00007fffefc47528 in raise () from /usr/lib/libc.so.6
(gdb) bt

+ Trace 235229

  • #0 raise
    from /usr/lib/libc.so.6
  • #1 abort
    from /usr/lib/libc.so.6
  • #2 g_assertion_message
    at gtestutils.c line 2356
  • #3 g_assertion_message_expr
    at gtestutils.c line 2371
  • #4 value_compare_real
    at value.c line 1363
  • #5 value_compare
    at value.c line 1454
  • #6 gnm_expr_eval
    at expr.c line 1270
  • #7 function_iterate_argument_values
    at func.c line 2361
  • #8 collect_floats
  • #9 float_range_function
  • #10 gnumeric_count
    at functions.c line 797
  • #11 function_call_with_exprs
    at func.c line 1879
  • #12 gnm_expr_eval
    at expr.c line 1453
  • #13 gnm_expr_eval
    at expr.c line 1368
  • #14 gnm_expr_top_eval
    at expr.c line 3124
  • #15 gnm_cell_eval_content
    at dependent.c line 1665
  • #16 cell_dep_eval
    at dependent.c line 1250
  • #17 dependent_eval
    at dependent.c line 1755
  • #18 workbook_recalc
    at dependent.c line 2869
  • #19 workbook_view_new_from_input
    at workbook-view.c line 1294
  • #20 workbook_view_new_from_uri
    at workbook-view.c line 1337
  • #21 convert
    at ssconvert.c line 721
  • #22 main
    at ssconvert.c line 913 

--
Juha Kylmänen
Comment 1 Jean Bréfort 2015-07-04 14:59:45 UTC 
I don't see the link to gtestutils, the crash occurs in value_compare_real() because one at least of the values is NULL:

+ Trace 235232

  • #0 __GI_raise
    at ../nptl/sysdeps/unix/sysv/linux/raise.c line 56
  • #1 __GI_abort
    at abort.c line 89
  • #2 g_assertion_message
    from /lib/x86_64-linux-gnu/libglib-2.0.so.0
  • #3 g_assertion_message_expr
    from /lib/x86_64-linux-gnu/libglib-2.0.so.0
  • #4 value_compare_real
    at value.c line 1363
  • #5 value_compare
    at value.c line 1454
  • #6 gnm_expr_eval
    at expr.c line 1270
  • #7 function_iterate_argument_values
    at func.c line 2361
  • #8 collect_floats
  • #9 float_range_function
    at collect.c line 626
  • #10 function_call_with_exprs
    at func.c line 1879
  • #11 gnm_expr_eval
    at expr.c line 1453
  • #12 gnm_expr_eval
    at expr.c line 1368
  • #13 gnm_expr_top_eval
    at expr.c line 3124
  • #14 gnm_cell_eval_content
    at dependent.c line 1665
  • #15 cell_dep_eval
    at dependent.c line 1250
  • #16 dependent_eval
    at dependent.c line 1755
  • #17 workbook_recalc
    at dependent.c line 2869
  • #18 workbook_view_new_from_input
    at workbook-view.c line 1294
  • #19 workbook_view_new_from_uri
    at workbook-view.c line 1337
  • #20 convert
    at ssconvert.c line 715
  • #21 main
    at ssconvert.c line 903 

Not sure how it should be fixed, probably returning an error when a or b is NULL.
Comment 2 Andreas J. Guelzow 2015-07-04 17:22:34 UTC 
I think the real issue is that we should never get to value_compare_real with NULL values.
Comment 3 Andreas J. Guelzow 2015-07-04 17:31:21 UTC 
I was wrong, Value_compare_real is clearly trying to handle the NULL values by translating it into VALUE_EMPTY.
Comment 4 Andreas J. Guelzow 2015-07-04 17:39:14 UTC 
To me this looks like a logic error. We have:
	
switch (PAIR (ta,tb)) {
	case CPAIR (VALUE_EMPTY,VALUE_EMPTY):
		g_assert_not_reached(); /* Should have hit trivial case.  */
		return IS_EQUAL;

The trivial case referred to is
	if (a == b)
		return IS_EQUAL;

If we have a NULL value and a non-null value that is VALUE_EMPTY, I don't see why we would always hit the 'trivial case", since just after testing for the trivial case we have:

	ta = VALUE_IS_EMPTY (a) ? VALUE_EMPTY : a->v_any.type;
	tb = VALUE_IS_EMPTY (b) ? VALUE_EMPTY : b->v_any.type;

I think we should just remove the g_assert_not_reached().

Am I missing something?
Comment 5 Morten Welinder 2015-07-04 21:53:02 UTC 
Andreas: I think you are right.  This code, in its current form, isn't
terribly old, so we probably just haven't exercised it enough.
Comment 6 Andreas J. Guelzow 2015-07-05 01:02:52 UTC 
This problem has been fixed in the unstable development version. The fix will be available in the next major software release. You may need to upgrade your Linux distribution to obtain that newer version.
Comment 7 Andreas J. Guelzow 2015-07-17 13:22:32 UTC 
*** Bug 749907 has been marked as a duplicate of this bug. ***