The 'key' parameter for 'des_set_key()' must be at least 8 bytes long.

besser82 / libxcrypt

Extended crypt library for descrypt, md5crypt, bcrypt, and others

GNU Lesser General Public License v2.1

189 stars 52 forks source link

The 'key' parameter for 'des_set_key()' must be at least 8 bytes long. #50

Closed besser82 closed 5 years ago

besser82 commented 5 years ago

Coverity scan complains that tainted data can be stored in 'rawkey1'. Since there are excactly 8 bytes of data accessed, we should enforce this as a minimum parameter length.

besser82 commented 5 years ago

The description of the issue says 'Untrusted array index read'. To me it sounds like it is concerned about the code may read out-of-bounds…

besser82 commented 5 years ago

@zackw I'll submit this PR to Coverity Scan before merging it. Let's see whether it changes that problem…

solardiz commented 5 years ago

Regarding the DES stuff, I previously (in e-mail a year ago) recommended switching to the FreeSec fork from musl (which I had put some effort into improving in multiple ways - lower memory needs, mostly glibc UFC-crypt compatible handling of invalid salts). I think this hasn't been done yet, or has it? While I don't mind addressing the individual Coverity issues in the current code, perhaps that will have to be re-done after the switch.

zackw commented 5 years ago

@solardiz No, that hasn't happened yet. I was going to do that but then I got buried under a mountain of other work and never got back to it.