* [PATCH v2 1/2] libselinux: ensure strlen() is not called on NULL
@ 2019-09-20 5:59 Nicolas Iooss
2019-09-20 5:59 ` [PATCH v2 2/2] libselinux: do not add rc to pos twice Nicolas Iooss
2019-09-20 14:22 ` [PATCH v2 1/2] libselinux: ensure strlen() is not called on NULL Stephen Smalley
0 siblings, 2 replies; 5+ messages in thread
From: Nicolas Iooss @ 2019-09-20 5:59 UTC (permalink / raw)
To: selinux
When compile_regex() calls regex_prepare_data() and this function fails
in the following condition:
*regex = regex_data_create();
if (!(*regex))
return -1;
... error_data has been zero-ed and compile_regex() calls:
regex_format_error(&error_data,
regex_error_format_buffer,
sizeof(regex_error_format_buffer));
This leads to a call to strlen(error_data->error_buffer), where
error_data->error_buffer is NULL.
Avoid this by checking that error_data->error_buffer is not NULL before
trying to format it.
This issue has been found using clang's static analyzer:
https://337-118970575-gh.circle-artifacts.com/0/output-scan-build/2019-09-01-181851-6152-1/report-0b122b.html#EndPath
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
---
libselinux/src/regex.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/libselinux/src/regex.c b/libselinux/src/regex.c
index a6fcbbfec1f3..c835dd1b0e5d 100644
--- a/libselinux/src/regex.c
+++ b/libselinux/src/regex.c
@@ -519,6 +519,29 @@ void regex_format_error(struct regex_error_data const *error_data, char *buffer,
if (pos >= buf_size)
goto truncated;
+ /* Return early if there is no error to format */
+#ifdef USE_PCRE2
+ if (!error_data->error_code) {
+ rc = snprintf(buffer + pos, buf_size - pos, "no error code");
+ if (rc < 0)
+ abort();
+ pos += rc;
+ if (pos >= buf_size)
+ goto truncated;
+ return;
+ }
+#else
+ if (!error_data->error_buffer) {
+ rc = snprintf(buffer + pos, buf_size - pos, "empty error");
+ if (rc < 0)
+ abort();
+ pos += rc;
+ if (pos >= buf_size)
+ goto truncated;
+ return;
+ }
+#endif
+
if (error_data->error_offset > 0) {
#ifdef USE_PCRE2
rc = snprintf(buffer + pos, buf_size - pos, "At offset %zu: ",
--
2.22.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] libselinux: do not add rc to pos twice
2019-09-20 5:59 [PATCH v2 1/2] libselinux: ensure strlen() is not called on NULL Nicolas Iooss
@ 2019-09-20 5:59 ` Nicolas Iooss
2019-09-20 14:22 ` Stephen Smalley
2019-09-23 13:54 ` Stephen Smalley
2019-09-20 14:22 ` [PATCH v2 1/2] libselinux: ensure strlen() is not called on NULL Stephen Smalley
1 sibling, 2 replies; 5+ messages in thread
From: Nicolas Iooss @ 2019-09-20 5:59 UTC (permalink / raw)
To: selinux
In regex_format_error(), when error_data->error_offset is zero, rc is
not updated and should not be added to pos again.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
---
libselinux/src/regex.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/libselinux/src/regex.c b/libselinux/src/regex.c
index c835dd1b0e5d..770bc3ea1310 100644
--- a/libselinux/src/regex.c
+++ b/libselinux/src/regex.c
@@ -552,10 +552,10 @@ void regex_format_error(struct regex_error_data const *error_data, char *buffer,
#endif
if (rc < 0)
abort();
+ pos += rc;
+ if (pos >= buf_size)
+ goto truncated;
}
- pos += rc;
- if (pos >= buf_size)
- goto truncated;
#ifdef USE_PCRE2
rc = pcre2_get_error_message(error_data->error_code,
--
2.22.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] libselinux: ensure strlen() is not called on NULL
2019-09-20 5:59 [PATCH v2 1/2] libselinux: ensure strlen() is not called on NULL Nicolas Iooss
2019-09-20 5:59 ` [PATCH v2 2/2] libselinux: do not add rc to pos twice Nicolas Iooss
@ 2019-09-20 14:22 ` Stephen Smalley
1 sibling, 0 replies; 5+ messages in thread
From: Stephen Smalley @ 2019-09-20 14:22 UTC (permalink / raw)
To: Nicolas Iooss, selinux
On 9/20/19 1:59 AM, Nicolas Iooss wrote:
> When compile_regex() calls regex_prepare_data() and this function fails
> in the following condition:
>
> *regex = regex_data_create();
> if (!(*regex))
> return -1;
>
> ... error_data has been zero-ed and compile_regex() calls:
>
> regex_format_error(&error_data,
> regex_error_format_buffer,
> sizeof(regex_error_format_buffer));
>
> This leads to a call to strlen(error_data->error_buffer), where
> error_data->error_buffer is NULL.
>
> Avoid this by checking that error_data->error_buffer is not NULL before
> trying to format it.
>
> This issue has been found using clang's static analyzer:
> https://337-118970575-gh.circle-artifacts.com/0/output-scan-build/2019-09-01-181851-6152-1/report-0b122b.html#EndPath
>
> Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Personally, I would have just used a single error message for both cases
(and only wrapped the condition itself with #ifdef...#else...#endif),
but I'm ok with this as well.
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
> ---
> libselinux/src/regex.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/libselinux/src/regex.c b/libselinux/src/regex.c
> index a6fcbbfec1f3..c835dd1b0e5d 100644
> --- a/libselinux/src/regex.c
> +++ b/libselinux/src/regex.c
> @@ -519,6 +519,29 @@ void regex_format_error(struct regex_error_data const *error_data, char *buffer,
> if (pos >= buf_size)
> goto truncated;
>
> + /* Return early if there is no error to format */
> +#ifdef USE_PCRE2
> + if (!error_data->error_code) {
> + rc = snprintf(buffer + pos, buf_size - pos, "no error code");
> + if (rc < 0)
> + abort();
> + pos += rc;
> + if (pos >= buf_size)
> + goto truncated;
> + return;
> + }
> +#else
> + if (!error_data->error_buffer) {
> + rc = snprintf(buffer + pos, buf_size - pos, "empty error");
> + if (rc < 0)
> + abort();
> + pos += rc;
> + if (pos >= buf_size)
> + goto truncated;
> + return;
> + }
> +#endif
> +
> if (error_data->error_offset > 0) {
> #ifdef USE_PCRE2
> rc = snprintf(buffer + pos, buf_size - pos, "At offset %zu: ",
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] libselinux: do not add rc to pos twice
2019-09-20 5:59 ` [PATCH v2 2/2] libselinux: do not add rc to pos twice Nicolas Iooss
@ 2019-09-20 14:22 ` Stephen Smalley
2019-09-23 13:54 ` Stephen Smalley
1 sibling, 0 replies; 5+ messages in thread
From: Stephen Smalley @ 2019-09-20 14:22 UTC (permalink / raw)
To: Nicolas Iooss, selinux
On 9/20/19 1:59 AM, Nicolas Iooss wrote:
> In regex_format_error(), when error_data->error_offset is zero, rc is
> not updated and should not be added to pos again.
>
> Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
> ---
> libselinux/src/regex.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/libselinux/src/regex.c b/libselinux/src/regex.c
> index c835dd1b0e5d..770bc3ea1310 100644
> --- a/libselinux/src/regex.c
> +++ b/libselinux/src/regex.c
> @@ -552,10 +552,10 @@ void regex_format_error(struct regex_error_data const *error_data, char *buffer,
> #endif
> if (rc < 0)
> abort();
> + pos += rc;
> + if (pos >= buf_size)
> + goto truncated;
> }
> - pos += rc;
> - if (pos >= buf_size)
> - goto truncated;
>
> #ifdef USE_PCRE2
> rc = pcre2_get_error_message(error_data->error_code,
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] libselinux: do not add rc to pos twice
2019-09-20 5:59 ` [PATCH v2 2/2] libselinux: do not add rc to pos twice Nicolas Iooss
2019-09-20 14:22 ` Stephen Smalley
@ 2019-09-23 13:54 ` Stephen Smalley
1 sibling, 0 replies; 5+ messages in thread
From: Stephen Smalley @ 2019-09-23 13:54 UTC (permalink / raw)
To: Nicolas Iooss, selinux
On 9/20/19 1:59 AM, Nicolas Iooss wrote:
> In regex_format_error(), when error_data->error_offset is zero, rc is
> not updated and should not be added to pos again.
Thanks, both patches applied.
>
> Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
> ---
> libselinux/src/regex.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/libselinux/src/regex.c b/libselinux/src/regex.c
> index c835dd1b0e5d..770bc3ea1310 100644
> --- a/libselinux/src/regex.c
> +++ b/libselinux/src/regex.c
> @@ -552,10 +552,10 @@ void regex_format_error(struct regex_error_data const *error_data, char *buffer,
> #endif
> if (rc < 0)
> abort();
> + pos += rc;
> + if (pos >= buf_size)
> + goto truncated;
> }
> - pos += rc;
> - if (pos >= buf_size)
> - goto truncated;
>
> #ifdef USE_PCRE2
> rc = pcre2_get_error_message(error_data->error_code,
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-09-23 13:54 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-20 5:59 [PATCH v2 1/2] libselinux: ensure strlen() is not called on NULL Nicolas Iooss
2019-09-20 5:59 ` [PATCH v2 2/2] libselinux: do not add rc to pos twice Nicolas Iooss
2019-09-20 14:22 ` Stephen Smalley
2019-09-23 13:54 ` Stephen Smalley
2019-09-20 14:22 ` [PATCH v2 1/2] libselinux: ensure strlen() is not called on NULL Stephen Smalley
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.