From fe06b4b836b3afd5b27039914dea1c7fe20bd78d Mon Sep 17 00:00:00 2001 From: Matthias Schiffer Date: Sat, 16 May 2020 18:53:40 +0200 Subject: [PATCH] usign-exec: improve usign -F output handling While not likely to happen in pratice, nothing guarantees that read() will retrieve more than 1 byte at a time. The easiest way to make this code compliant is to wrap the file descriptor using fdopen(). While we're at it, also - remove useless memset() - check fingerprint for validity The check is particularly relevant, as a usign bug [1] causing short fingerprint outputs only went unnoticed for so long because the trailing newline was considered one of the 16 characters ucert was expecting. [1] https://patchwork.ozlabs.org/project/openwrt/patch/8ead1fd6a61117b54b4efd5111fe0d19e4eef9c5.1589642591.git.mschiffer@universe-factory.net/ Signed-off-by: Matthias Schiffer --- usign-exec.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/usign-exec.c b/usign-exec.c index 241d630..482e510 100644 --- a/usign-exec.c +++ b/usign-exec.c @@ -93,6 +93,7 @@ int usign_s(const char *msgfile, const char *seckeyfile, const char *sigfile, bo */ static int usign_f(char fingerprint[17], const char *pubkeyfile, const char *seckeyfile, const char *sigfile, bool quiet) { int fds[2]; + FILE *f; pid_t pid; int status; const char *usign_argv[16] = {0}; @@ -141,17 +142,22 @@ static int usign_f(char fingerprint[17], const char *pubkeyfile, const char *sec waitpid(pid, &status, 0); status = WIFEXITED(status) ? WEXITSTATUS(status) : -1; - if (fingerprint && !status) { - ssize_t r; - memset(fingerprint, 0, 17); - r = read(fds[0], fingerprint, 17); - if (r < 16) - status = -1; + if (!fingerprint || status) { + close(fds[0]); + return status; + } - fingerprint[16] = '\0'; + f = fdopen(fds[0], "r"); + if (fread(fingerprint, 1, 16, f) != 16) + status = -1; + fclose(f); + if (status) + return status; + + fingerprint[16] = '\0'; + if (strspn(fingerprint, "0123456789abcdefABCDEF") != 16) + status = -1; - } - close(fds[0]); return status; } -- 2.25.1