Re: argp: pass NULL as msgid to dgettext without checks

2019-01-05 Thread He X
prove that man-db and tar works(which used to segfault). hope builtin argp
could be updated soon. i will report if i met another NULL pointer case.

thanks for your work! :)

Bruno Haible  于2019年1月6日周日 上午12:17写道:

> He X wrote:
> > > The msgid argument is a null-terminated string.
> >
> > problem is that msgid is a empty pointer, not a pointer to a
> > NULL-terminated string.
> >
> > full backtrace:
> >
> > ```
> > #0  0x77fab65d in strcmp (l=0x0, r=0x77eff7df "Packaged by
> > %s\n") at src/string/strcmp.c:5
> > #1  0x77f51351 in __mo_lookup (p=0x77efa000, size=55838,
> s=0x0)
> > at src/locale/__mo_lookup.c:25
> > #2  0x77f51e14 in dcngettext (domainname=0x77ffe320 "tar",
> > msgid1=0x0, msgid2=0x0, n=1, category=5) at src/locale/dcngettext.c:211
> > #3  0x77f52204 in dgettext (domainname=0x0, msgid=0x0) at
> > src/locale/dcngettext.c:271
> > #4  0x5558a63c in argp_args_usage (argp=argp@entry
> =0x7fffe740,
> > state=state@entry=0x7fffe8f0, levels=levels@entry=0x7fffe690,
> > advance=advance@entry=1, stream=stream@entry=0x555c5e20) at
> > argp-help.c:1415
> > #5  0x5558bcdd in _help (argp=0x7fffe740,
> > state=state@entry=0x7fffe8f0,
> > stream=0x77ffb2a0 , flags=flags@entry=634,
> > name=0x7fffed94 "tar") at argp-help.c:1640
> > #6  0x5558ca08 in argp_state_help (state=state@entry
> =0x7fffe8f0,
> > stream=, flags=flags@entry=634) at argp-help.c:1741
> > #7  0x5558d088 in argp_default_parser (key=,
> > arg=, state=0x7fffe8f0) at argp-parse.c:96
> > #8  0x5558df89 in group_parse (arg=, key=63,
> > state=0x7fffe8f0, group=0x555c40b0) at argp-parse.c:234
> > #9  parser_parse_opt (val=, opt=50331711,
> > parser=0x7fffe880) at argp-parse.c:749
> > #10 parser_parse_next (arg_ebadkey=,
> > parser=0x7fffe880) at argp-parse.c:860
> > #11 argp_parse (argp=, argc=2, argv=,
> > flags=, end_index=0x7fffe9f0, input=0x555c3500
> > )
> > at argp-parse.c:928
> > #12 0xd3b7 in decode_options (argv=0x7fffffffeb58, argc=2) at
> > tar.c:2312
> > #13 main (argc=, argv=) at tar.c:2698
> > ```
>
> Thank you! With this stack trace, I'm fixing the issue as follows:
>
>
> 2019-01-05  Bruno Haible  
>
> argp: Don't pass an invalid argument to dgettext().
> Reported by He X .
> * lib/argp.h (struct argp): Clarify that the args_doc field may be
> NULL.
> * lib/argp-help.c (argp_args_usage): Don't pass a NULL args_doc to
> dgettext().
>
> diff --git a/lib/argp.h b/lib/argp.h
> index 317ac03..7aba887 100644
> --- a/lib/argp.h
> +++ b/lib/argp.h
> @@ -69,6 +69,9 @@ typedef int error_t;
>  extern "C" {
>  #endif
>
> +/* Glibc documentation:
> +   https://www.gnu.org/software/libc/manual/html_node/Argp.html */
> +
>  /* A description of a particular option.  A pointer to an array of
> these is passed in the OPTIONS field of an argp structure.  Each option
> entry can correspond to one long option and/or one short option; more
> @@ -236,9 +239,9 @@ struct argp
>   ARGP_KEY_ definitions below.  */
>argp_parser_t parser;
>
> -  /* A string describing what other arguments are wanted by this
> program.  It
> - is only used by argp_usage to print the "Usage:" message.  If it
> - contains newlines, the strings separated by them are considered
> +  /* If non-NULL, a string describing what other arguments are wanted by
> this
> + program.  It is only used by argp_usage to print the "Usage:"
> message.
> + If it contains newlines, the strings separated by them are considered
>   alternative usage patterns, and printed on separate lines (lines
> after
>   the first are prefix by "  or: " instead of "Usage:").  */
>const char *args_doc;
> diff --git a/lib/argp-help.c b/lib/argp-help.c
> index e5375a0..e5e97ec 100644
> --- a/lib/argp-help.c
> +++ b/lib/argp-help.c
> @@ -1412,8 +1412,10 @@ argp_args_usage (const struct argp *argp, const
> struct argp_state *state,
>char *our_level = *levels;
>int multiple = 0;
>const struct argp_child *child = argp->children;
> -  const char *tdoc = dgettext (argp->argp_domain, argp->args_doc), *nl =
> 0;
> +  const char *tdoc =
> +argp->args_doc ? dgettext (argp->argp_domain, argp->args_doc) : NULL;
>const char *fdoc = filter_doc (tdoc, ARGP_KEY_HELP_ARGS_DOC, argp,
> state);
> +  const char *nl = NULL;
>
>

argp: pass NULL as msgid to dgettext without checks

2018-12-22 Thread He X
since my libc is musl, tar is using builtin argp implementation, which will
pass NULL as msgid to dgettext without additional checks.

but according to
https://www.gnu.org/software/gettext/manual/html_node/Interface-to-gettext.html,
passing NULL to gettext is undefined. and, musl chose to crash..

could you please check if msgid is NULL before passing it to dgettext? the
problem code is located at argp-help.c.

 related links: https://www.openwall.com/lists/musl/2017/03/24/10.

-- 
Best regards,
xhe