[packagekit] make security-check

[Richard Hughes]

I normally check all the project code using flawfinder to find the big
obvious bugs. I've ignored a ton in PackageKit (where we check return
values or lengths and so isn't a problem) but here are the results from
the backends:

./backends/poldek/pk-backend-poldek.c:199:  [4] (race) access:
  This usually indicates a security flaw.  If an attacker can change
  anything along the path between the call to access() and the file's actual
  use (e.g., by moving files), the attacker can exploit the race
  condition. Set up the correct permissions (e.g., using setuid()) and try to
  open the file directly. 
./backends/alpm/pk-backend-alpm.c:621:  [2] (buffer) char:
  Statically-sized arrays can be overflowed. Perform bounds checking,
  use functions that limit length, or ensure that the size is larger than
  the maximum possible length. 
./backends/alpm/pk-backend-alpm.c:632:  [2] (misc) fopen:
  Check when opening files - can an attacker redirect it (via symlinks),
  force the opening of special file type (e.g., device files), move
  things around to create a race condition, control its ancestors, or change
  its contents?. 
./backends/alpm/pk-backend-alpm.c:520:  [1] (buffer) strlen:
  Does not handle strings that are not \0-terminated (it could cause a
  crash if unprotected). 
./backends/alpm/pk-backend-alpm.c:526:  [1] (buffer) strlen:
  Does not handle strings that are not \0-terminated (it could cause a
  crash if unprotected). 
./backends/alpm/pk-backend-alpm.c:544:  [1] (buffer) strlen:
  Does not handle strings that are not \0-terminated (it could cause a
  crash if unprotected). 
./backends/alpm/pk-backend-alpm.c:548:  [1] (buffer) strncat:
  Easily used incorrectly (e.g., incorrectly computing the correct
  maximum size to add). Consider strlcat or automatically resizing strings. 
./backends/alpm/pk-backend-alpm.c:563:  [1] (buffer) strlen:
  Does not handle strings that are not \0-terminated (it could cause a
  crash if unprotected). 
./backends/alpm/pk-backend-alpm.c:563:  [1] (buffer) strlen:
  Does not handle strings that are not \0-terminated (it could cause a
  crash if unprotected). 
./backends/alpm/pk-backend-alpm.c:570:  [1] (buffer) strlen:
  Does not handle strings that are not \0-terminated (it could cause a
  crash if unprotected). 
./backends/alpm/pk-backend-alpm.c:650:  [1] (buffer) strlen:
  Does not handle strings that are not \0-terminated (it could cause a
  crash if unprotected). 
./backends/alpm/pk-backend-alpm.c:656:  [1] (buffer) strlen:
  Does not handle strings that are not \0-terminated (it could cause a
  crash if unprotected). 
./backends/alpm/pk-backend-alpm.c:664:  [1] (buffer) strlen:
  Does not handle strings that are not \0-terminated (it could cause a
  crash if unprotected). 
./backends/alpm/pk-backend-alpm.c:666:  [1] (buffer) strlen:
  Does not handle strings that are not \0-terminated (it could cause a
  crash if unprotected). 
./backends/poldek/pk-backend-poldek.c:230:  [1] (buffer) strlen:
  Does not handle strings that are not \0-terminated (it could cause a
  crash if unprotected). 
./backends/poldek/pk-backend-poldek.c:732:  [1] (buffer) strlen:
  Does not handle strings that are not \0-terminated (it could cause a
  crash if unprotected). 
./backends/poldek/pk-backend-poldek.c:744:  [1] (buffer) strlen:
  Does not handle strings that are not \0-terminated (it could cause a
  crash if unprotected). 
./backends/poldek/pk-backend-poldek.c:1495:  [1] (buffer) strlen:
  Does not handle strings that are not \0-terminated (it could cause a
  crash if unprotected). 

If you _KNOW_ that the code is secure and couldn't be used to crash the
daemon just add a debug statement on the previous line, something like;

/* ITS4: ignore, this is just debug text */

Some of the flaws seem big and scary and probably just need some bounds
checking in the right places.

Richard.