diff mbox series

[v2,1/2] libselinux: ensure strlen() is not called on NULL

Message ID 20190920055955.2780-1-nicolas.iooss@m4x.org (mailing list archive)
State Accepted
Headers show
Series [v2,1/2] libselinux: ensure strlen() is not called on NULL | expand

Commit Message

Nicolas Iooss Sept. 20, 2019, 5:59 a.m. UTC
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(+)

Comments

Stephen Smalley Sept. 20, 2019, 2:22 p.m. UTC | #1
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: ",
>
diff mbox series

Patch

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: ",