[PATCH] common: state: check length

[PATCH] common: state: check length

[Jan Remmet]

if written_length is read from a partial written bucket it may be to
big and xmalloc will panic barebox.

Check if the value is sane.

Signed-off-by: Jan Remmet <j.remmet@phytec.de>
---
 common/state/backend_bucket_direct.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/common/state/backend_bucket_direct.c b/common/state/backend_bucket_direct.c
index 95ddb9310685..fc633eea8dec 100644
--- a/common/state/backend_bucket_direct.c
+++ b/common/state/backend_bucket_direct.c
@@ -67,6 +67,10 @@ static int state_backend_bucket_direct_read(struct state_backend_storage_bucket
 	}
 	if (meta.magic == direct_magic) {
 		read_len = meta.written_length;
+		if (read_len < 0 || read_len > direct->max_size) {
+			dev_err(direct->dev, "Wrong length in meta data\n");
+			return -EINVAL;
+		}
 	} else {
 		if (meta.magic != ~0 && !!meta.magic)
 			bucket->wrong_magic = 1;
-- 
2.7.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] common: state: check length

[Sascha Hauer]

On Tue, May 21, 2019 at 04:44:13PM +0200, Jan Remmet wrote:
> if written_length is read from a partial written bucket it may be to
> big and xmalloc will panic barebox.
> 
> Check if the value is sane.
> 
> Signed-off-by: Jan Remmet <j.remmet@phytec.de>
> ---
>  common/state/backend_bucket_direct.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/common/state/backend_bucket_direct.c b/common/state/backend_bucket_direct.c
> index 95ddb9310685..fc633eea8dec 100644
> --- a/common/state/backend_bucket_direct.c
> +++ b/common/state/backend_bucket_direct.c
> @@ -67,6 +67,10 @@ static int state_backend_bucket_direct_read(struct state_backend_storage_bucket
>  	}
>  	if (meta.magic == direct_magic) {
>  		read_len = meta.written_length;
> +		if (read_len < 0 || read_len > direct->max_size) {

written_length in struct state_backend_storage_bucket_direct_meta is
unsigned and so should be read_len. Then you can skip the read_len < 0
check.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2] common: state: check length

[Jan Remmet]

if written_length is read from a partial written bucket it may be to
big and xmalloc will panic barebox.

Check if the value is sane. Make read_len unsigned to avoid negative
values.

Signed-off-by: Jan Remmet <j.remmet@phytec.de>
---
v2: replace compare < 0 by making read_len unsigned

 common/state/backend_bucket_direct.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/common/state/backend_bucket_direct.c b/common/state/backend_bucket_direct.c
index 95ddb9310685..0dbd334db821 100644
--- a/common/state/backend_bucket_direct.c
+++ b/common/state/backend_bucket_direct.c
@@ -52,7 +52,7 @@ static int state_backend_bucket_direct_read(struct state_backend_storage_bucket
 	struct state_backend_storage_bucket_direct *direct =
 	    get_bucket_direct(bucket);
 	struct state_backend_storage_bucket_direct_meta meta;
-	ssize_t read_len;
+	uint32_t read_len;
 	void *buf;
 	int ret;
 
@@ -67,6 +67,11 @@ static int state_backend_bucket_direct_read(struct state_backend_storage_bucket
 	}
 	if (meta.magic == direct_magic) {
 		read_len = meta.written_length;
+		if (read_len > direct->max_size) {
+			dev_err(direct->dev, "Wrong length in meta data\n");
+			return -EINVAL;
+
+		}
 	} else {
 		if (meta.magic != ~0 && !!meta.magic)
 			bucket->wrong_magic = 1;
-- 
2.7.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2] common: state: check length

[Sascha Hauer]

On Thu, May 23, 2019 at 09:49:00AM +0200, Jan Remmet wrote:
> if written_length is read from a partial written bucket it may be to
> big and xmalloc will panic barebox.
> 
> Check if the value is sane. Make read_len unsigned to avoid negative
> values.
> 
> Signed-off-by: Jan Remmet <j.remmet@phytec.de>
> ---
> v2: replace compare < 0 by making read_len unsigned

Applied, thanks

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox