lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <30d65ea9170d4f60bd76ed516541cb46@AcuMS.aculab.com> Date: Thu, 25 May 2023 09:14:53 +0000 From: David Laight <David.Laight@...LAB.COM> To: 'Kenny Ho' <y2kenny@...il.com>, Andrew Lunn <andrew@...n.ch> CC: Marc Dionne <marc.dionne@...istor.com>, Kenny Ho <Kenny.Ho@....com>, "David Howells" <dhowells@...hat.com>, "David S. Miller" <davem@...emloft.net>, "Eric Dumazet" <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, "linux-afs@...ts.infradead.org" <linux-afs@...ts.infradead.org>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org> Subject: RE: [PATCH] Remove hardcoded static string length From: Kenny Ho > Sent: 24 May 2023 19:01 > > On Wed, May 24, 2023 at 1:43 PM Andrew Lunn <andrew@...n.ch> wrote: > > > > The other end of the socket should not blow up, because that would be > > an obvious DOS or buffer overwrite attack vector. So you need to > > decide, do you want to expose such issues and see if anything does > > actually blow up, or do you want to do a bit more work and correctly > > terminate the string when capped? > > Right... I guess it's not clear to me that existing implementations > null-terminate correctly when UTS_RELEASE causes the string to exceed > the 65 byte size of rxrpc_version_string. We can of course do better, > but I hesitate to do strncpy because I am not familiar with this code > base enough to tell if this function is part of some hot path where > strncpy matters. The whole thing looks like it is expecting a max of 64 characters and a terminating '\0'. Since UTE_RELEASE goes in between two fixed strings truncating the whole thing to 64/65 chars/bytes doesn't seem ideal. I does rather beg the question as what is in UTS_RELEASE when it exceeds (IIRC) about 48 characters? If UTS_RELEASE is getting that long, it might easily exceed the 64 characters returned by uname(). I suspect that you need to truncate UTS_RELEASE to limit the string to 64 characters - so something like: static char id[65]; if (!id[0]) snprintf(id, sizeof id, "xxx-%.48s-yyy", UTS_RELEASE); Using an on-stack buffer almost certainly wouldn't matter. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Powered by blists - more mailing lists