Dear OpenBSD community, I've just stumbled across a malfunction in signify: It cannot handle file names that contain a `)` character, when checking a list of hashes generated by `sha256` command line utilities (`sha256sum --tags` on Linux).
To blame for this is this simple statement in signify.c:681 rv = sscanf(line, "%31s (%1023[^)]) = %223s", c.algo, c.file, c.hash); which doesn't really like a filename 'parentheses)' like in SHA256 (parentheses)) = e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 As far as I can tell this does *NOT* lead to security issues – you can only get false check failures, you can't trick signify by having a file named `totallylegit) = hashaftermodification`. I'm attaching a patch. In order to remove the `^)`, I had to look for the *last* occurence of `)`; that required breaking up the single `sscanf` into multiple steps (finding the hash algorithm specification, identifying opening and closing (), extracting the file name between, and finally extracting the hash). This patch applies to out-of-tree signify[1] and current openbsd/src master[2]. It's only tested to compile and work under the former, though. Excuse my lack of a running OpenBSD. I use signify on Linux. Hope this is helpful, best regards, Marcus M [1]https://github.com/aperezdc/signify [2]openbsd/src: aad52b956f3c9d6dc166616cb67359458b6ada01 diff --git signify.c signify.c index 6a9660fb24f..215d9acb781 100644 --- signify.c +++ signify.c @@ -656,9 +656,9 @@ verifychecksums(char *msg, int argc, char **argv, int quiet) struct ohash_info info = { 0, NULL, ecalloc, efree, NULL }; struct ohash myh; struct checksum c; - char *e, *line, *endline; + char *e, *line, *endline, *parenthesis; int hasfailed = 0; - int i, rv; + int i, rv, len; unsigned int slot; ohash_init(&myh, 6, &info); @@ -678,10 +678,32 @@ verifychecksums(char *msg, int argc, char **argv, int quiet) #if PATH_MAX < 1024 || HASHBUFSIZE < 224 #error sizes are wrong #endif - rv = sscanf(line, "%31s (%1023[^)]) = %223s", - c.algo, c.file, c.hash); - if (rv != 3) - errx(1, "unable to parse checksum line %s", line); + /* Look for well-formed algorithm first */ + rv = sscanf(line, "%31s ", c.algo); + if (rv != 1) + errx(1, "unable to parse checksum type in in %s", line); + line += strlen(c.algo); + /* skip whitespace */ + while ((*line) && (*line == ' ' || *line == '\t')) + line++; + /* Look for opening and closing parenthesis */ + parenthesis = strchr(line, '('); + if (!parenthesis) + errx(1, "unable to find beginning of filename in %s", line); + line = ++parenthesis; + parenthesis = strrchr(line, ')'); + if (!parenthesis) + errx(1, "unable to find end of filename in %s", line); + len = parenthesis - line; + /* copy over filename - might contain ")" */ + memset(c.file + len, 0, PATH_MAX - len); + memcpy(c.file, line, len); + /* extract hash */ + line = parenthesis + 1; + rv = sscanf(line, " = %223s", c.hash); + if (rv != 1) + errx(1, "unable to extract hash from %s", line); + line = endline; if (argc) {