Re: argp: pass NULL as msgid to dgettext without checks
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=0x7fffeb58, 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;
>
>if (fdoc)
> {
>
>
> > you could see that from #3, msgid is 0x0.
> >
> > > I don't see which dgettext invocation in argp-help.c is the one that
> > needs the
> > NULL check.
> >
> > it's not one but multiple. argp->doc[argp_doc()], and
> > argp->args_doc[argp_args_usage[]) are what i have found . i am not sure
> if
> > there's any other, but i'll give the fix a test and see.
>
> Ah right. The 'doc' field of a 'struct argp' can be NULL as well, this is
> documented in
>
Re: argp: pass NULL as msgid to dgettext without checks
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=0x7fffeb58, 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;
if (fdoc)
{
> you could see that from #3, msgid is 0x0.
>
> > I don't see which dgettext invocation in argp-help.c is the one that
> needs the
> NULL check.
>
> it's not one but multiple. argp->doc[argp_doc()], and
> argp->args_doc[argp_args_usage[]) are what i have found . i am not sure if
> there's any other, but i'll give the fix a test and see.
Ah right. The 'doc' field of a 'struct argp' can be NULL as well, this is
documented in
https://www.gnu.org/software/libc/manual/html_node/Argp-Parsers.html.
The 'doc' field in a 'struct argp_option' looks like it can be NULL as well,
see argp-help.c line 1178.
2019-01-05 Bruno Haible
argp: Don't pass an invalid argument to dgettext().
Reported by He X .
* lib/argp-help.c (print_header, argp_doc): Don't pass a NULL doc to
dgettext().
diff --git a/lib/argp-help.c b/lib/argp-help.c
index e5e97ec..75abe84 100644
--- a/lib/argp-help.c
+++ b/lib/argp-help.c
@@
Re: argp: pass NULL as msgid to dgettext without checks
Hi, He X wrote on 2018-12-22: > according to > https://www.gnu.org/software/gettext/manual/html_node/Interface-to-gettext.html, > passing NULL to gettext is undefined. Likewise the LI18NUX 2000 specification (p. 39) says "The msgid argument is a null-terminated string." And likewise the LSB 3.0 https://refspecs.linuxfoundation.org/LSB_3.0.0/LSB-PDA/LSB-PDA/baselib-dgettext.html > since my libc is musl, tar is using builtin argp implementation, which will > pass NULL as msgid to dgettext without additional checks. > > > could you please check if msgid is NULL before passing it to dgettext? the > problem code is located at argp-help.c. I don't see which dgettext invocation in argp-help.c is the one that needs the NULL check. Can you please provide a stack trace (from gdb, from a 'tar' program built with CFLAGS="-ggdb") of the crash? Bruno
argp: pass NULL as msgid to dgettext without checks
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
