Re: [PATCH] mount.cifs: fix buffer overrun in set_password

Enzo Matsumiya
2026-04-01T13:40:49Z

Hi Dave,

On 03/31, David Disseldorp wrote:
>The existing (j > pass_length) check is insufficient to avoid dst buffer
>overrun into the start of the adjacent struct parsed_mount_info field.
>Check for overrun before writing to dst, and account for comma-expansion
>and null-termination.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=16044
>
>Reported-by: Bruno Bierbaumer <bruno@bierbaumer.net>
>Signed-off-by: David Disseldorp <ddiss@suse.de>
>---
> mount.cifs.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/mount.cifs.c b/mount.cifs.c
>index 1923913..d41ca6a 100644
>--- a/mount.cifs.c
>+++ b/mount.cifs.c
>@@ -350,13 +350,13 @@ set_password(struct parsed_mount_info *parsed_info, const char *src,
> unsigned int i = 0, j = 0;
>
> while (src[i]) {
>- if (src[i] == ',')
>- dst[j++] = ',';
>- dst[j++] = src[i++];
>- if (j > pass_length) {
>+ if (j + 2 >= pass_length) {

There is the overrun bug, yes, but unconditionally accounting for comma
expansion here will crop the password if it has at least 1 comma and
(unparsed) length == MOUNT_PASSWD_SIZE, e.g.:

(password here is <511 * 'a'> + ',')

# echo -e "username=administrator\npassword=$(printf 'a%.0s' {1..511})," > /tmp/creds
# ./mount.cifs -o credentials=/tmp/creds //w25.vm.test/test /mnt/test
Converted password too long!
error 1 (Operation not permitted) opening credential file /tmp/creds

Maybe password/password2 could be malloc'd with size
"strlen(src) + <number of commas in src>" and bounds checked against that?
Just a thought.

Cheers,

Enzo