Re: Bug#866890: pspp - cve-2017-10791 - cve-2017-10792

[Friedrich Beckmann]

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

[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

[Friedrich Beckmann]

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

[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.

 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

[Friedrich Beckmann]

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