session: detect short read of /dev/urandom in rpc_random()

rpc_random() stored the fread() result in an int and only checked for
"ret < 0".  fread() returns a size_t and never reports a negative value,
so the check was dead code: a short read (or a 0-byte read on a failing
/dev/urandom) was silently accepted.  In that case the 16-byte buffer is
only partially filled and the remaining bytes stay zero, yielding a
weak and in the worst case fully predictable (all-zero) session id.

Use a size_t and require the whole buffer to be filled, returning an
error otherwise so the caller aborts session creation.

Assisted-by: Claude:claude-opus-4-8
Link: https://github.com/openwrt/rpcd/pull/34
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>

1 files changed, 7 insertions, 3 deletions

diff --git a/session.c b/session.c
index 6ac226a..f56f2bf 100644
--- a/session.c
+++ b/session.c
@@ -152,7 +152,7 @@ rpc_random(char *dest)
 	unsigned char buf[16] = { 0 };
 	FILE *f;
 	int i;
-	int ret;
+	size_t ret;
 
 	f = fopen("/dev/urandom", "r");
 	if (!f)
@@ -161,8 +161,12 @@ rpc_random(char *dest)
 	ret = fread(buf, 1, sizeof(buf), f);
 	fclose(f);
 
-	if (ret < 0)
-		return ret;
+	/* fread() returns a size_t, so a short read can never be negative.
+	 * Require the full buffer to be filled, otherwise we would derive the
+	 * session id from (partially) uninitialized/zero data, resulting in a
+	 * predictable identifier. */
+	if (ret != sizeof(buf))
+		return -1;
 
 	for (i = 0; i < sizeof(buf); i++)
 		sprintf(dest + (i<<1), "%02x", buf[i]);