Comment 2 for bug 1039636

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Security review:
 * Hardening options enabled, but not with PIE. Builds without errors or warnings. Lintian clean, no setuid, fscaps or privileged operations via sudo/su/pkexec. No dbus, initscripts/upstart jobs or cron jobs. Simple C program with a pam configuration file
 * code audit of socket-sucker.c:
  * handles environ fine
  * does not check the return value of snprintf and may not be nul terminated. Specifically, this:
snprintf(serv_addr.sun_path, sizeof(serv_addr.sun_path), "%s/%s", home, ".freerdp-socket");
Should be changed to:
if (snprintf(serv_addr.sun_path, sizeof(serv_addr.sun_path) - 1, "%s/%s", home, ".freerdp-socket") > sizeof(serv_addr.sun_path) - 1) {
    return -1;
}
  * not checking return code early enough for read() which could mean the write call writes something unintended
  * not checking return code of write() at all.

Tyler will comment on the pam configuration since he is looking at libpam-freerdp. Otherwise, conditional ACK provided the above code changes are made and it is compiled with PIE.