* [commit] objc-lang.c: avoid string overrun
@ 2011-02-28 4:10 Michael Snyder
2011-02-28 4:52 ` Jan Kratochvil
2011-02-28 12:21 ` Pedro Alves
0 siblings, 2 replies; 6+ messages in thread
From: Michael Snyder @ 2011-02-28 4:10 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 0 bytes --]
[-- Attachment #2: strncpy2.txt --]
[-- Type: text/plain, Size: 726 bytes --]
2011-02-27 Michael Snyder <msnyder@vmware.com>
* objc-lang.c (selectors_info): Prevent string overrun.
Index: objc-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/objc-lang.c,v
retrieving revision 1.91
diff -u -p -u -p -r1.91 objc-lang.c
--- objc-lang.c 10 Jan 2011 20:38:49 -0000 1.91
+++ objc-lang.c 28 Feb 2011 02:13:37 -0000
@@ -720,7 +720,7 @@ selectors_info (char *regexp, int from_t
strcpy(myregexp, ".*]");
else
{
- strcpy(myregexp, regexp);
+ strncpy(myregexp, regexp, sizeof (myregexp) - 1);
if (myregexp[strlen(myregexp) - 1] == '$') /* end of selector */
myregexp[strlen(myregexp) - 1] = ']'; /* end of method name */
else
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [commit] objc-lang.c: avoid string overrun
2011-02-28 4:10 [commit] objc-lang.c: avoid string overrun Michael Snyder
@ 2011-02-28 4:52 ` Jan Kratochvil
2011-02-28 18:02 ` Michael Snyder
2011-02-28 12:21 ` Pedro Alves
1 sibling, 1 reply; 6+ messages in thread
From: Jan Kratochvil @ 2011-02-28 4:52 UTC (permalink / raw)
To: Michael Snyder; +Cc: gdb-patches
Hi Michael,
On Mon, 28 Feb 2011 03:15:47 +0100, Michael Snyder wrote:
> --- objc-lang.c 10 Jan 2011 20:38:49 -0000 1.91
> +++ objc-lang.c 28 Feb 2011 02:13:37 -0000
char myregexp[2048];
> @@ -720,7 +720,7 @@ selectors_info (char *regexp, int from_t
> strcpy(myregexp, ".*]");
> else
> {
> - strcpy(myregexp, regexp);
> + strncpy(myregexp, regexp, sizeof (myregexp) - 1);
> if (myregexp[strlen(myregexp) - 1] == '$') /* end of selector */
> myregexp[strlen(myregexp) - 1] = ']'; /* end of method name */
> else
I agree it fixes a bug. But still if the limit applies then the immediately
following strlen will read uninitialized memory myregexp[2047].
Do you agree with this fix instead?
(Yes, the code should be completely different but we fix only bugs now.)
Thanks,
Jan
gdb/
2011-02-28 Jan Kratochvil <jan.kratochvil@redhat.com>
* objc-lang.c (selectors_info): Error on too long REGEXP.
--- a/gdb/objc-lang.c
+++ b/gdb/objc-lang.c
@@ -720,7 +720,9 @@ selectors_info (char *regexp, int from_tty)
strcpy(myregexp, ".*]");
else
{
- strncpy(myregexp, regexp, sizeof (myregexp) - 1);
+ if (sizeof (myregexp) < strlen (regexp) + 1)
+ error (_("Regexp is too long: %s"), regexp);
+ strcpy(myregexp, regexp);
if (myregexp[strlen(myregexp) - 1] == '$') /* end of selector */
myregexp[strlen(myregexp) - 1] = ']'; /* end of method name */
else
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [commit] objc-lang.c: avoid string overrun
2011-02-28 4:10 [commit] objc-lang.c: avoid string overrun Michael Snyder
2011-02-28 4:52 ` Jan Kratochvil
@ 2011-02-28 12:21 ` Pedro Alves
2011-02-28 18:27 ` Michael Snyder
1 sibling, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2011-02-28 12:21 UTC (permalink / raw)
To: gdb-patches; +Cc: Michael Snyder
On Monday 28 February 2011 02:15:47, Michael Snyder wrote:
> {
> - strcpy(myregexp, regexp);
> + strncpy(myregexp, regexp, sizeof (myregexp) - 1);
Such fixes ain't that much better. At the bare least, you'd
need to null terminate the result, as strncpy does not do that
for you if REGEXP is large enough --- strncpy was not
designed as a safe strcpy. And then proceeding as if nothing
happened when `myregexp' isn't large enough is just b0rked.
> if (myregexp[strlen(myregexp) - 1] == '$') /* end of selector */
> myregexp[strlen(myregexp) - 1] = ']'; /* end of method name */
--
Pedro Alves
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [commit] objc-lang.c: avoid string overrun
2011-02-28 4:52 ` Jan Kratochvil
@ 2011-02-28 18:02 ` Michael Snyder
2011-02-28 18:15 ` Jan Kratochvil
0 siblings, 1 reply; 6+ messages in thread
From: Michael Snyder @ 2011-02-28 18:02 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches
Jan Kratochvil wrote:
> Hi Michael,
>
> On Mon, 28 Feb 2011 03:15:47 +0100, Michael Snyder wrote:
>> --- objc-lang.c 10 Jan 2011 20:38:49 -0000 1.91
>> +++ objc-lang.c 28 Feb 2011 02:13:37 -0000
> char myregexp[2048];
>> @@ -720,7 +720,7 @@ selectors_info (char *regexp, int from_t
>> strcpy(myregexp, ".*]");
>> else
>> {
>> - strcpy(myregexp, regexp);
>> + strncpy(myregexp, regexp, sizeof (myregexp) - 1);
>> if (myregexp[strlen(myregexp) - 1] == '$') /* end of selector */
>> myregexp[strlen(myregexp) - 1] = ']'; /* end of method name */
>> else
>
> I agree it fixes a bug. But still if the limit applies then the immediately
> following strlen will read uninitialized memory myregexp[2047].
>
> Do you agree with this fix instead?
>
> (Yes, the code should be completely different but we fix only bugs now.)
OK, please apply.
> gdb/
> 2011-02-28 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * objc-lang.c (selectors_info): Error on too long REGEXP.
>
> --- a/gdb/objc-lang.c
> +++ b/gdb/objc-lang.c
> @@ -720,7 +720,9 @@ selectors_info (char *regexp, int from_tty)
> strcpy(myregexp, ".*]");
> else
> {
> - strncpy(myregexp, regexp, sizeof (myregexp) - 1);
> + if (sizeof (myregexp) < strlen (regexp) + 1)
> + error (_("Regexp is too long: %s"), regexp);
> + strcpy(myregexp, regexp);
> if (myregexp[strlen(myregexp) - 1] == '$') /* end of selector */
> myregexp[strlen(myregexp) - 1] = ']'; /* end of method name */
> else
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [commit] objc-lang.c: avoid string overrun
2011-02-28 18:02 ` Michael Snyder
@ 2011-02-28 18:15 ` Jan Kratochvil
0 siblings, 0 replies; 6+ messages in thread
From: Jan Kratochvil @ 2011-02-28 18:15 UTC (permalink / raw)
To: Michael Snyder; +Cc: gdb-patches
On Mon, 28 Feb 2011 19:00:29 +0100, Michael Snyder wrote:
> Jan Kratochvil wrote:
> >gdb/
> >2011-02-28 Jan Kratochvil <jan.kratochvil@redhat.com>
> >
> > * objc-lang.c (selectors_info): Error on too long REGEXP.
>
> OK, please apply.
Checked in:
http://sourceware.org/ml/gdb-cvs/2011-02/msg00235.html
Thanks,
Jan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [commit] objc-lang.c: avoid string overrun
2011-02-28 12:21 ` Pedro Alves
@ 2011-02-28 18:27 ` Michael Snyder
0 siblings, 0 replies; 6+ messages in thread
From: Michael Snyder @ 2011-02-28 18:27 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves wrote:
> On Monday 28 February 2011 02:15:47, Michael Snyder wrote:
>> {
>> - strcpy(myregexp, regexp);
>> + strncpy(myregexp, regexp, sizeof (myregexp) - 1);
>
> Such fixes ain't that much better. At the bare least, you'd
> need to null terminate the result, as strncpy does not do that
> for you if REGEXP is large enough --- strncpy was not
> designed as a safe strcpy. And then proceeding as if nothing
> happened when `myregexp' isn't large enough is just b0rked.
>
>> if (myregexp[strlen(myregexp) - 1] == '$') /* end of selector */
>> myregexp[strlen(myregexp) - 1] = ']'; /* end of method name */
>
Yes, Jan provided a more complete fix.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-02-28 18:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-28 4:10 [commit] objc-lang.c: avoid string overrun Michael Snyder
2011-02-28 4:52 ` Jan Kratochvil
2011-02-28 18:02 ` Michael Snyder
2011-02-28 18:15 ` Jan Kratochvil
2011-02-28 12:21 ` Pedro Alves
2011-02-28 18:27 ` Michael Snyder
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).