pspp-dev
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Possible bug in pc+-file-reader.c ??


From: John Darrington
Subject: Possible bug in pc+-file-reader.c ??
Date: Sat, 12 Sep 2015 09:19:18 +0200
User-agent: Mutt/1.5.21 (2010-09-15)

I think the implementation of pcp_detect is buggy.
Amoungst others, valgrind is complaining about this code:



/* Returns true if FILE is an SPSS/PC+ system file,
   false otherwise. */
static int
pcp_detect (FILE *file)
{
  static const char signature[4] = "SPSS";
  char buf[sizeof signature];

  if (fseek (file, 0x104, SEEK_SET)
      || (fread (buf, sizeof buf, 1, file) != 1 && !feof (file)))
    return -errno;

  return !memcmp (buf, signature, sizeof buf);
}


There are several issues I see here:

If the "return -errno" path is taken, then the comment seems to be incorrect.
Assuming that errno is non-zero the return value is "true" which cannot be 
correct.
fread will return 0 meaning that the comparison 0 != 1 will 


Secondly, the POSIX specification for fseek says this:

 "The fseek() function allows the file-position indicator to be set beyond the 
  end of existing data in the file."

It seems that the code has assumed fseek will fail if the file is 
less than 0x104 bytes long.  So in my case (I have a short file)
fseek succeeds, fread fails (returns 0) .  Hence memcmp is getting
called when buf is uninitialized.


Is my analysis correct or have I missed something?

J'


-- 
Avoid eavesdropping.  Send strong encryted 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.

Attachment: signature.asc
Description: Digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]