Re: pspp - cve-2017-10791 - cve-2017-10792
I applied fixes for both of these bugs to the PSPP repository, as the following commits. The fixes will be in the next PSPP release. commit 41c6f5447941e5d36d0554ba874671649353752f Author: Ben PfaffDate: Tue Jul 4 12:58:55 2017 -0400 sys-file-reader: Fix integer overflows in parse_long_string_missing_values(). Crafted system files caused integer overflow errors that in turn caused aborts. This fixes the problem. CVE-2017-10791. See also https://bugzilla.redhat.com/show_bug.cgi?id=1467004. See also https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=866890. See also https://security-tracker.debian.org/tracker/CVE-2017-10791. Found by team OWL337, using the collAFL fuzzer. commit bf03b53a3c0f0d1066062f37919015a8fa6ad436 Author: Ben Pfaff Date: Tue Jul 4 12:54:47 2017 -0400 sys-file-reader: Avoid null dereference skipping bad extension record 18. read_record() assumed that read_extension_record() never set its output argument to NULL when it returned true, but this is possible in an error case. CVE-2017-10792. See also https://bugzilla.redhat.com/show_bug.cgi?id=1467005. See also https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=866890. See also https://security-tracker.debian.org/tracker/CVE-2017-10792. Reported by team OWL337, with fuzzer collAFL.
Re: Bug#866890: pspp - cve-2017-10791 - cve-2017-10792
Hi Ben, my understanding is that they bring up two different problems. For https://bugzilla.redhat.com/show_bug.cgi?id=1467004 (Hash Function) the argument is that shift operations and overflows are undefined or implementation dependent for signed integers as used in the hash function. https://www.securecoding.cert.org/confluence/display/c/INT13-C.+Use+bitwise+operators+only+on+unsigned+operands Shifting a negative number is „bad“ by that definition and that is what they checked. But when looking at the code, isn’t there a problem when a pointer is cast to integer on 64 Bit platforms because the pointer is 64 Bit and the integer is 32 Bit in hash_pointer? Wouldn’t we want to have a hash based on the 64 Bit as for hash_double? For https://bugzilla.redhat.com/show_bug.cgi?id=1467005 (crash on csv conversion) they managed to generate a file which results in a crash when analyzed. Although pspp stills gives an error message that something is wrong in the file… Friedrich > Am 04.07.2017 um 15:27 schrieb Ben Pfaff: > > The attribution of the problem to the hash function is probably wrong, > since that function is purely combinatorial logic, but the report as a > whole is right because the attachment in the bug report at > https://bugzilla.redhat.com/show_bug.cgi?id=1467004 does cause > pspp-convert to assert-fail. > > I'm looking into it. > > On Mon, Jul 03, 2017 at 08:50:56PM +0200, John Darrington wrote: >> I suspect this report is mistaken. But this bit is Ben's code, so I'll let >> him comment on >> that. >> >> J' >> >> On Mon, Jul 03, 2017 at 07:22:57AM +0200, Friedrich Beckmann wrote: >> Dear owl337 team, >> >> thanks for looking at pspp and finding the security problems >> >> https://security-tracker.debian.org/tracker/CVE-2017-10791 >> >> and >> >> https://security-tracker.debian.org/tracker/CVE-2017-10792 >> >> in pspp! Your reports are quite detailed. Could you describe how you >> found the problems, i.e. do >> you have some information about collAFL? >> >> Regards >> >> Friedrich >> >> >> >> ___ >> pspp-dev mailing list >> pspp-...@gnu.org >> https://lists.gnu.org/mailman/listinfo/pspp-dev >> >> -- >> Avoid eavesdropping. Send strong encrypted email. >> PGP Public key ID: 1024D/2DE827B3 >> fingerprint = 8797 A26D 0854 2EAB 0285 A290 8A67 719C 2DE8 27B3 >> See http://sks-keyservers.net or any PGP keyserver for public key. >> > >
Re: Bug#866890: pspp - cve-2017-10791 - cve-2017-10792
The attribution of the problem to the hash function is probably wrong, since that function is purely combinatorial logic, but the report as a whole is right because the attachment in the bug report at https://bugzilla.redhat.com/show_bug.cgi?id=1467004 does cause pspp-convert to assert-fail. I'm looking into it. On Mon, Jul 03, 2017 at 08:50:56PM +0200, John Darrington wrote: > I suspect this report is mistaken. But this bit is Ben's code, so I'll let > him comment on > that. > > J' > > On Mon, Jul 03, 2017 at 07:22:57AM +0200, Friedrich Beckmann wrote: > Dear owl337 team, > > thanks for looking at pspp and finding the security problems > > https://security-tracker.debian.org/tracker/CVE-2017-10791 > > and > > https://security-tracker.debian.org/tracker/CVE-2017-10792 > > in pspp! Your reports are quite detailed. Could you describe how you > found the problems, i.e. do > you have some information about collAFL? > > Regards > > Friedrich > > > > ___ > pspp-dev mailing list > pspp-...@gnu.org > https://lists.gnu.org/mailman/listinfo/pspp-dev > > -- > Avoid eavesdropping. Send strong encrypted email. > PGP Public key ID: 1024D/2DE827B3 > fingerprint = 8797 A26D 0854 2EAB 0285 A290 8A67 719C 2DE8 27B3 > See http://sks-keyservers.net or any PGP keyserver for public key. >
Re: Bug#866890: pspp - cve-2017-10791 - cve-2017-10792
Hi John, > Am 04.07.2017 um 07:10 schrieb John Darrington: > > On Mon, Jul 03, 2017 at 11:37:30PM +0200, Friedrich Beckmann wrote: > Hi John, > > today I looked a little bit at the hash function. I think the problem is > that compared to > the referenced code the x parameter is type int instead of unsigned int. > Googling around the > overflow behavior of signed and the shift right of signed is not defined > in the c standard > although ???many?" implementations assume 2th complement signed > implementation. Both is well > defined for unsigned int operations. > > Ahh. Perhaps you're right. But I cannot see that this would cause a crash, > so I suspect that's > another problem. They compiled with a compiler switch -fsanitized=undefined. I assume that this produces the crash. > I changed the parameter type from int to unsigned int and I cannot see a > problem in the regression. > > What problems did you encounter before your change (if any)? I encountered no problems. At first I assumed that they use some form of static code analysis. Then I tried to run our regression with the above mentioned switch but on MacOS I encountered some compile problems. In my view the behavior in our code might produce a bad hash as it deviates from the original code as the right shift is different for int and unsigned int. But I cannot see how this produces a security vulnerability. Friedrich signature.asc Description: Message signed with OpenPGP using GPGMail
Re: Bug#866890: pspp - cve-2017-10791 - cve-2017-10792
On Mon, Jul 03, 2017 at 11:37:30PM +0200, Friedrich Beckmann wrote: Hi John, today I looked a little bit at the hash function. I think the problem is that compared to the referenced code the x parameter is type int instead of unsigned int. Googling around the overflow behavior of signed and the shift right of signed is not defined in the c standard although ???many?" implementations assume 2th complement signed implementation. Both is well defined for unsigned int operations. Ahh. Perhaps you're right. But I cannot see that this would cause a crash, so I suspect that's another problem. I changed the parameter type from int to unsigned int and I cannot see a problem in the regression. What problems did you encounter before your change (if any)? But looking at the code I wondered if this hash function also works on 64 Bit architectures. The reference only talks about uint32_t. I cannot see that it wouldn't "work". But it might not create such an efficient hash. Anyway maybe Ben will be able to have a look soon. J' -- Avoid eavesdropping. Send strong encrypted email. PGP Public key ID: 1024D/2DE827B3 fingerprint = 8797 A26D 0854 2EAB 0285 A290 8A67 719C 2DE8 27B3 See http://sks-keyservers.net or any PGP keyserver for public key. signature.asc Description: Digital signature
Re: Bug#866890: pspp - cve-2017-10791 - cve-2017-10792
Hi John, today I looked a little bit at the hash function. I think the problem is that compared to the referenced code the x parameter is type int instead of unsigned int. Googling around the overflow behavior of signed and the shift right of signed is not defined in the c standard although „many?" implementations assume 2th complement signed implementation. Both is well defined for unsigned int operations. I changed the parameter type from int to unsigned int and I cannot see a problem in the regression. But looking at the code I wondered if this hash function also works on 64 Bit architectures. The reference only talks about uint32_t. Regards Friedrich > Am 03.07.2017 um 20:50 schrieb John Darrington: > > I suspect this report is mistaken. But this bit is Ben's code, so I'll let > him comment on > that. > > J' > > On Mon, Jul 03, 2017 at 07:22:57AM +0200, Friedrich Beckmann wrote: > Dear owl337 team, > > thanks for looking at pspp and finding the security problems > > https://security-tracker.debian.org/tracker/CVE-2017-10791 > > and > > https://security-tracker.debian.org/tracker/CVE-2017-10792 > > in pspp! Your reports are quite detailed. Could you describe how you > found the problems, i.e. do > you have some information about collAFL? > > Regards > > Friedrich > > signature.asc Description: Message signed with OpenPGP using GPGMail
Re: pspp - cve-2017-10791 - cve-2017-10792
I suspect this report is mistaken. But this bit is Ben's code, so I'll let him comment on that. J' On Mon, Jul 03, 2017 at 07:22:57AM +0200, Friedrich Beckmann wrote: Dear owl337 team, thanks for looking at pspp and finding the security problems https://security-tracker.debian.org/tracker/CVE-2017-10791 and https://security-tracker.debian.org/tracker/CVE-2017-10792 in pspp! Your reports are quite detailed. Could you describe how you found the problems, i.e. do you have some information about collAFL? Regards Friedrich ___ pspp-dev mailing list pspp-...@gnu.org https://lists.gnu.org/mailman/listinfo/pspp-dev -- Avoid eavesdropping. Send strong encrypted email. PGP Public key ID: 1024D/2DE827B3 fingerprint = 8797 A26D 0854 2EAB 0285 A290 8A67 719C 2DE8 27B3 See http://sks-keyservers.net or any PGP keyserver for public key. signature.asc Description: Digital signature
pspp - cve-2017-10791 - cve-2017-10792
Dear owl337 team, thanks for looking at pspp and finding the security problems https://security-tracker.debian.org/tracker/CVE-2017-10791 and https://security-tracker.debian.org/tracker/CVE-2017-10792 in pspp! Your reports are quite detailed. Could you describe how you found the problems, i.e. do you have some information about collAFL? Regards Friedrich