fread() - possible division by zero

fread() - possible division by zero

[Geraldo Netto]

[-- Attachment #1.1: Type: text/plain, Size: 1129 bytes --]

Dear Friends,

It seems we may have the same division by zero issue on fread():

This is the original code:

size_t fread(void *restrict destv, size_t size, size_t nmemb, FILE *restrict f)
{
	unsigned char *dest = destv;
	size_t len = size*nmemb, l = len, k;
	if (!size) nmemb = 0;

	FLOCK(f);

	f->mode |= f->mode-1;

	if (f->rend - f->rpos > 0) {
		/* First exhaust the buffer. */
		k = MIN(f->rend - f->rpos, l);
		memcpy(dest, f->rpos, k);
		f->rpos += k;
		dest += k;
		l -= k;
	}
	
	/* Read the remainder directly */
	for (; l; l-=k, dest+=k) {
		k = __toread(f) ? 0 : f->read(f, dest, l);
		if (k+1<=1) {
			FUNLOCK(f);
			return (len-l)/size;
		}
	}

	FUNLOCK(f);
	return nmemb;
}




It seems we need to check the variable size on return because if size is
zero
We'll have a division by zero and a segmentation fault

I'm sending the attached patch that changes the return as follows:

return (len-l)/(size != 0 ? size : 1);


I don't know if this is the correct approach, so, feel free to
change/let me know how to fix :)
Hope it helps


Kind Regards,

Geraldo Netto
Sapere Aude => Non dvcor, dvco
http://exdev.sf.net/

[-- Attachment #1.2: Type: text/html, Size: 1715 bytes --]

[-- Attachment #2: 0001-fread-avoid-possible-division-by-zero-when-size-0.patch --]
[-- Type: application/octet-stream, Size: 755 bytes --]

From 8acbbd4c8c768ec7a718f1040f45d7dfceb97283 Mon Sep 17 00:00:00 2001
From: geraldo netto <geraldonetto@gmail.com>
Date: Wed, 14 Feb 2018 16:46:13 -0200
Subject: [PATCH] fread(): avoid possible division by zero when size = 0

Signed-off-by: geraldo netto <geraldonetto@gmail.com>
---
 src/stdio/fread.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/stdio/fread.c b/src/stdio/fread.c
index aef75f7..1085d2a 100644
--- a/src/stdio/fread.c
+++ b/src/stdio/fread.c
@@ -27,7 +27,7 @@ size_t fread(void *restrict destv, size_t size, size_t nmemb, FILE *restrict f)
 		k = __toread(f) ? 0 : f->read(f, dest, l);
 		if (k+1<=1) {
 			FUNLOCK(f);
-			return (len-l)/size;
+			return (len-l)/(size != 0 ? size : 1);
 		}
 	}
 
-- 
2.7.4

Re: fread() - possible division by zero

[Markus Wichmann]

On Wed, Feb 14, 2018 at 04:50:44PM -0200, Geraldo Netto wrote:
> Dear Friends,
> 
> It seems we may have the same division by zero issue on fread():
> 
> This is the original code:
> 
> size_t fread(void *restrict destv, size_t size, size_t nmemb, FILE *restrict f)
> {
> 	unsigned char *dest = destv;
> 	size_t len = size*nmemb, l = len, k;
> 	if (!size) nmemb = 0;
> 
> 	FLOCK(f);
> 
> 	f->mode |= f->mode-1;
> 
> 	if (f->rend - f->rpos > 0) {
> 		/* First exhaust the buffer. */
> 		k = MIN(f->rend - f->rpos, l);
> 		memcpy(dest, f->rpos, k);
> 		f->rpos += k;
> 		dest += k;
> 		l -= k;
> 	}
> 	
> 	/* Read the remainder directly */
> 	for (; l; l-=k, dest+=k) {
> 		k = __toread(f) ? 0 : f->read(f, dest, l);
> 		if (k+1<=1) {
> 			FUNLOCK(f);
> 			return (len-l)/size;
> 		}
> 	}
> 
> 	FUNLOCK(f);
> 	return nmemb;
> }
> 
> 
> 
> 
> It seems we need to check the variable size on return because if size is
> zero
> We'll have a division by zero and a segmentation fault
> 

If size is zero, then l is zero. Even with a filled read buffer, that
can't change since l is lower than all possible other unsigned values,
thus k will be zero as well. So at the "for" loop, l will be zero, thus
the loop will never be entered, thus the return statement is unreachable
in this case. So again, no division will take place, least of all one by
zero.

Say, did you even test these cases? It would take all of five minutes to
do, and belay your suspicions quicker than I can write these responses.

HTH,
Markus

Re: fread() - possible division by zero

[Szabolcs Nagy]

* Geraldo Netto <geraldonetto@gmail.com> [2018-02-14 16:50:44 -0200]:
> It seems we may have the same division by zero issue on fread():
> 

it's good practice to actually test your theory,
in this case it's just a fread(d,0,n,f) call.

then you could see there is no issue, the code works fine.