All of lore.kernel.org
 help / color / mirror / Atom feed
From: Norbert Manthey <nmanthey@amazon.de>
To: <xen-devel@lists.xenproject.org>
Cc: Ian Jackson <iwj@xenproject.org>, Juergen Gross <jgross@suse.com>,
	Wei Liu <wl@xen.org>, Julien Grall <jgrall@amazon.co.uk>,
	Michael Kurth <mku@amazon.de>,
	Norbert Manthey <nmanthey@amazon.de>
Subject: [PATCH XENSTORE v1 06/10] xenstored: handle port reads correctly
Date: Fri, 26 Feb 2021 15:41:40 +0100	[thread overview]
Message-ID: <20210226144144.9252-7-nmanthey@amazon.de> (raw)
In-Reply-To: <20210226144144.9252-1-nmanthey@amazon.de>

The read value could be larger than a signed 32bit integer. As -1 is
used as error value, we should not rely on using the full 32 bits.
Hence, when reading the port number, we should make sure we only return
valid values.

This change sanity checks the input.
The issue is that the value for the port is
 1. transmitted as a string, with a fixed amount of digits.
 2. Next, this string is parsed by a function that can deal with strings
    representing 64bit integers
 3. A 64bit integer is returned, and will be truncated to it's lower
    32bits, resulting in a wrong port number (in case the sender of the
    string decides to craft a suitable 64bit value).

The value is typically provided by the kernel, which has this value hard
coded in the proper range. As we use the function strtoul, non-digit
character are considered as end of the input, and hence do not require
checking. Therefore, this change only covers the corner case to make
sure we stay in the 32 bit range.

This bug was discovered and resolved using Coverity Static Analysis
Security Testing (SAST) by Synopsys, Inc.

Signed-off-by: Norbert Manthey <nmanthey@amazon.de>
Reviewed-by: Thomas Friebel <friebelt@amazon.de>
Reviewed-by: Julien Grall <jgrall@amazon.co.uk>

---
 tools/xenstore/xenstored_posix.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/xenstore/xenstored_posix.c b/tools/xenstore/xenstored_posix.c
--- a/tools/xenstore/xenstored_posix.c
+++ b/tools/xenstore/xenstored_posix.c
@@ -116,7 +116,7 @@ evtchn_port_t xenbus_evtchn(void)
 {
 	int fd;
 	int rc;
-	evtchn_port_t port;
+	uint64_t port;
 	char str[20];
 
 	fd = open(XENSTORED_PORT_DEV, O_RDONLY);
@@ -136,6 +136,10 @@ evtchn_port_t xenbus_evtchn(void)
 	port = strtoul(str, NULL, 0);
 
 	close(fd);
+
+	if (port >= UINT32_MAX)
+		return -1;
+
 	return port;
 }
 
-- 
2.17.1




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879





  parent reply	other threads:[~2021-02-26 14:42 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-26 14:41 [PATCH XENSTORE v1 00/10] Code analysis fixes Norbert Manthey
2021-02-26 14:41 ` [PATCH XENSTORE v1 01/10] xenstore: add missing NULL check Norbert Manthey
2021-03-02  5:07   ` Jürgen Groß
2021-03-03 16:07   ` Ian Jackson
2021-02-26 14:41 ` [PATCH XENSTORE v1 02/10] xenstore: fix print format string Norbert Manthey
2021-03-02  5:07   ` Jürgen Groß
2021-03-03 16:08   ` Ian Jackson
2021-02-26 14:41 ` [PATCH XENSTORE v1 03/10] xenstore: check formats of trace Norbert Manthey
2021-03-02  5:08   ` Jürgen Groß
2021-03-03 16:08   ` Ian Jackson
2021-02-26 14:41 ` [PATCH XENSTORE v1 04/10] xenstore_client: handle memory on error Norbert Manthey
2021-03-02  5:08   ` Jürgen Groß
2021-03-03 16:10   ` Ian Jackson
2021-02-26 14:41 ` [PATCH XENSTORE v1 05/10] xenstore: handle daemon creation errors Norbert Manthey
2021-03-02  5:08   ` Jürgen Groß
2021-03-03 16:10   ` Ian Jackson
2021-02-26 14:41 ` Norbert Manthey [this message]
2021-02-26 15:36   ` [PATCH XENSTORE v1 06/10] xenstored: handle port reads correctly Andrew Cooper
2021-03-02  5:15     ` Jürgen Groß
2021-03-02  7:48       ` Norbert Manthey
2021-03-02  8:12         ` Jürgen Groß
2021-02-26 14:41 ` [PATCH XENSTORE v1 07/10] xenstore: handle do_mkdir and do_rm failure Norbert Manthey
2021-03-02  5:09   ` Jürgen Groß
2021-03-03 16:11   ` Ian Jackson
2021-02-26 14:41 ` [PATCH XENSTORE v1 08/10] xenstore: add missing NULL check Norbert Manthey
2021-03-02  5:10   ` Jürgen Groß
2021-03-03 16:11   ` Ian Jackson
2021-02-26 14:41 ` [PATCH XENSTORE v1 09/10] xs: handle daemon socket error Norbert Manthey
2021-03-02  5:10   ` Jürgen Groß
2021-03-03 16:13   ` Ian Jackson
2021-03-04 14:58     ` Norbert Manthey
2021-03-04 15:04       ` Ian Jackson
2021-02-26 14:41 ` [PATCH XENSTORE v1 10/10] xs: add error handling Norbert Manthey
2021-02-26 14:53   ` Julien Grall
2021-02-26 15:26     ` Norbert Manthey
2021-03-02  5:11   ` Jürgen Groß
2021-03-03 16:13   ` Ian Jackson
2021-03-01 18:01 ` [for-4.15] Re: [PATCH XENSTORE v1 00/10] Code analysis fixes Julien Grall
2021-03-01 19:39   ` Andrew Cooper
2021-03-02 10:04     ` Julien Grall
2021-03-03 18:45 ` Julien Grall

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210226144144.9252-7-nmanthey@amazon.de \
    --to=nmanthey@amazon.de \
    --cc=iwj@xenproject.org \
    --cc=jgrall@amazon.co.uk \
    --cc=jgross@suse.com \
    --cc=mku@amazon.de \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.