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) {
