[PATCH] cifskey: better use snprintf()

[Sebastian Krahmer]

Prefer snprintf() over sprintf() in cifskey.c
Projects that fork the code (pam_cifscreds) can't rely on
the max-size parameters.

Signed-off-by: Sebastian Krahmer <krahmer-***@public.gmane.org>
---


--- cifskey.c.orig 2014-04-08 13:10:41.653435040 +0200
+++ cifskey.c 2014-04-14 11:19:07.000118206 +0200
@@ -29,7 +29,8 @@
{
char desc[INET6_ADDRSTRLEN + sizeof(KEY_PREFIX) + 4];

- sprintf(desc, "%s:%c:%s", KEY_PREFIX, keytype, addr);
+ if (snprintf(desc, sizeof(desc), "%s:%c:%s", KEY_PREFIX, keytype, addr) >= (int)sizeof(desc))
+ return -1;

return keyctl_search(DEST_KEYRING, CIFS_KEY_TYPE, desc, 0);
}
@@ -38,15 +39,18 @@
key_serial_t
key_add(const char *addr, const char *user, const char *pass, char keytype)
{
- int len;
+ int len = 0;
char desc[INET6_ADDRSTRLEN + sizeof(KEY_PREFIX) + 4];
char val[MOUNT_PASSWD_SIZE + MAX_USERNAME_SIZE + 2];

/* set key description */
- sprintf(desc, "%s:%c:%s", KEY_PREFIX, keytype, addr);
+ if (snprintf(desc, sizeof(desc), "%s:%c:%s", KEY_PREFIX, keytype, addr) >= (int)sizeof(desc))
+ return -1;

/* set payload contents */
- len = sprintf(val, "%s:%s", user, pass);
+ len = snprintf(val, sizeof(val), "%s:%s", user, pass);
+ if (len >= (int)sizeof(val))
+ return -1;

return add_key(CIFS_KEY_TYPE, desc, val, len + 1, DEST_KEYRING);
}

[Jeff Layton]

On Mon, 14 Apr 2014 11:39:41 +0200

Post by Sebastian Krahmer
Prefer snprintf() over sprintf() in cifskey.c
Projects that fork the code (pam_cifscreds) can't rely on
the max-size parameters.
---
--- cifskey.c.orig 2014-04-08 13:10:41.653435040 +0200
+++ cifskey.c 2014-04-14 11:19:07.000118206 +0200
@@ -29,7 +29,8 @@
{
char desc[INET6_ADDRSTRLEN + sizeof(KEY_PREFIX) + 4];
- sprintf(desc, "%s:%c:%s", KEY_PREFIX, keytype, addr);
+ if (snprintf(desc, sizeof(desc), "%s:%c:%s", KEY_PREFIX, keytype, addr) >= (int)sizeof(desc))
+ return -1;
return keyctl_search(DEST_KEYRING, CIFS_KEY_TYPE, desc, 0);
}
@@ -38,15 +39,18 @@
key_serial_t
key_add(const char *addr, const char *user, const char *pass, char keytype)
{
- int len;
+ int len = 0;

This initialization doesn't appear to be needed.

Post by Sebastian Krahmer
char desc[INET6_ADDRSTRLEN + sizeof(KEY_PREFIX) + 4];
char val[MOUNT_PASSWD_SIZE + MAX_USERNAME_SIZE + 2];
/* set key description */
- sprintf(desc, "%s:%c:%s", KEY_PREFIX, keytype, addr);
+ if (snprintf(desc, sizeof(desc), "%s:%c:%s", KEY_PREFIX, keytype, addr) >= (int)sizeof(desc))
+ return -1;
/* set payload contents */
- len = sprintf(val, "%s:%s", user, pass);
+ len = snprintf(val, sizeof(val), "%s:%s", user, pass);
+ if (len >= (int)sizeof(val))
+ return -1;
return add_key(CIFS_KEY_TYPE, desc, val, len + 1, DEST_KEYRING);
}

I think we probably need to clean up the error handling in the callers
of these functions, but that's a separate problem. I'll go ahead and
merge this, and we can do that in a separate patch.

[Jeff Layton]

On Mon, 14 Apr 2014 11:39:41 +0200

Post by Sebastian Krahmer
Prefer snprintf() over sprintf() in cifskey.c
Projects that fork the code (pam_cifscreds) can't rely on
the max-size parameters.
---
--- cifskey.c.orig 2014-04-08 13:10:41.653435040 +0200
+++ cifskey.c 2014-04-14 11:19:07.000118206 +0200
@@ -29,7 +29,8 @@
{
char desc[INET6_ADDRSTRLEN + sizeof(KEY_PREFIX) + 4];
- sprintf(desc, "%s:%c:%s", KEY_PREFIX, keytype, addr);
+ if (snprintf(desc, sizeof(desc), "%s:%c:%s", KEY_PREFIX, keytype, addr) >= (int)sizeof(desc))
+ return -1;
return keyctl_search(DEST_KEYRING, CIFS_KEY_TYPE, desc, 0);
}
@@ -38,15 +39,18 @@
key_serial_t
key_add(const char *addr, const char *user, const char *pass, char keytype)
{
- int len;
+ int len = 0;
char desc[INET6_ADDRSTRLEN + sizeof(KEY_PREFIX) + 4];
char val[MOUNT_PASSWD_SIZE + MAX_USERNAME_SIZE + 2];
/* set key description */
- sprintf(desc, "%s:%c:%s", KEY_PREFIX, keytype, addr);
+ if (snprintf(desc, sizeof(desc), "%s:%c:%s", KEY_PREFIX, keytype, addr) >= (int)sizeof(desc))
+ return -1;
/* set payload contents */
- len = sprintf(val, "%s:%s", user, pass);
+ len = snprintf(val, sizeof(val), "%s:%s", user, pass);
+ if (len >= (int)sizeof(val))
+ return -1;
return add_key(CIFS_KEY_TYPE, desc, val, len + 1, DEST_KEYRING);
}

Slightly modified version merged. Thanks for the patch!