On Tue, 19 Sep 2017, Jacob Zimmermann - Sense of Security wrote: > Dear OpenBSD devs, > > As part of our ongoing information security research into software > vulnerabilities Sense of Security has identified a security > vulnerability in OpenBSDÂ’s lpr daemon. > > Sense of Security is an advocate of full disclosure and intends to > publish a security advisory for this vulnerability in accordance with > our vulnerability disclosure policy found here: > http://www.senseofsecurity.com.au/research/vulnerability-disclosure-policy. > This security advisory will be posted to all major security > vulnerability mailing lists and on our web site.
Thank you for your report! This message was sent to <[email protected]>, a public mailing list which is archived in various public sites. If this was meant as non-public disclosure, then it should have been sent, per the "Reporting problems" section of https://www.openbsd.org/security.html to <[email protected]> > The vulnerability is a variant of a previously known race-condition, in > which the attacker proceeded as follows: ... > The mechanism to prevent this attack currently involves storing the > printed file’s inode number in the CF file and ensuring that it has not > changed before the actual printing starts. > > However this remains vulnerable. The bug occurs in > usr.sbin/lpr/lpd/printjob.c, line 428: > > fino = i; > > where “fino” is the inode number extracted from the print job’s CF > file, with which the actual file’s inode number is compared. As of > revision 1.58 available in the OpenBSD CVS, the variable “i” is > declared “int” (line 337), so it remains a 32-bit integer even on > amd64 systems (among other 64 bit architectures). Consequently the inode > number comparison for symlink print jobs is effectively truncated to 32 > bits. Yep, that's a bug, invoking undefined behavior with arithmetic overflow as a bonus. Yay? Note that the same overflow issue exists in sendit(). (If you used tooling to locate the reported issue but it missed that case then you should review why.) ... > One of the possible solutions is to parse the inode number from the CF > file using a 64-bit accumulator variable instead of “i”, or to declare > “i” as unsigned long long. Simply removing the intermediate variables seems to be the simplest solution in both cases. Philip Guenther <[email protected]> Index: lpd/printjob.c =================================================================== RCS file: /data/src/openbsd/src/usr.sbin/lpr/lpd/printjob.c,v retrieving revision 1.58 diff -u -p -r1.58 printjob.c --- lpd/printjob.c 22 Nov 2016 16:03:57 -0000 1.58 +++ lpd/printjob.c 20 Sep 2017 03:54:42 -0000 @@ -417,15 +417,13 @@ printit(char *file) case 'S': cp = line+1; - i = 0; + fdev = 0; while (*cp >= '0' && *cp <= '9') - i = i * 10 + (*cp++ - '0'); - fdev = i; + fdev = fdev * 10 + (*cp++ - '0'); cp++; - i = 0; + fino = 0; while (*cp >= '0' && *cp <= '9') - i = i * 10 + (*cp++ - '0'); - fino = i; + fino = fino * 10 + (*cp++ - '0'); continue; case 'J': @@ -826,15 +824,13 @@ sendit(char *file) again: if (line[0] == 'S') { cp = line+1; - i = 0; + fdev = 0; while (*cp >= '0' && *cp <= '9') - i = i * 10 + (*cp++ - '0'); - fdev = i; + fdev = fdev * 10 + (*cp++ - '0'); cp++; - i = 0; + fino = 0; while (*cp >= '0' && *cp <= '9') - i = i * 10 + (*cp++ - '0'); - fino = i; + fino = fino * 10 + (*cp++ - '0'); continue; } if (line[0] >= 'a' && line[0] <= 'z') {
