Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8a Buffer Overflow #392

Closed
gopherbot opened this issue Dec 8, 2009 · 6 comments
Closed

8a Buffer Overflow #392

gopherbot opened this issue Dec 8, 2009 · 6 comments

Comments

@gopherbot
Copy link

by x41@freeshell.org:

Exploiting #221 in a different way:
Fresh install of:

Last version of Golang
cb140bac9ab0 release.2009-11-12/release

Tested on Ubuntu 9.04 and Ubuntu 9.10
XXXX@XXXX:~/go_src/src$ uname -a
Linux XXXX 2.6.28-11-generic #42-Ubuntu SMP Fri Apr 17 01:57:59 UTC 2009
i686 GNU/Linux

Attached Valgrind report and GDB report.

Binary: 8a

go_src/src/lib9/main.c

Breakpoint 1, main (argc=0x2, argv=0xbfdd72d4) at
/home/dsdsds/go_src/src/lib9/main.c:35
35      p9main(argc, argv);
gdb> list
30  extern void p9main(int, char**);
31  
32  int
33  main(int argc, char **argv)
34  {
35      p9main(argc, argv);
#######################################
              HERE
36      exits("main");
37      return 99;
              HERE
#######################################
38  }

Run it
r `python -c 'print "\x41"*99 + "\x42"'`
Just.. 99 + 1

Thanks guys!

Attachments:

  1. debuginfo1.tar.gz (2029 bytes)
@gopherbot
Copy link
Author

Comment 1 by x41@freeshell.org:

/usr/include/bits/string3.h:106: warning: 
call to __builtin___strcpy_chk will always overflow destination buffer
And this i intentionally not allowed with -D_FORTIFY_SOURCE=2, which doesn't allow
crossing field boundaries for str*/stp* etc. functions (and still allows that
for mem* etc.).
If we use -00 the problem is resolved, but if we really need to use -02 or -03
we have to edit Make.conf and modify like this:
 
CFLAGS=-ggdb -I$(GOROOT)/include -O2 -fno-inline -D_FORTIFY_SOURCE=1
The difference between -D_FORTIFY_SOURCE=1 and -D_FORTIFY_SOURCE=2
is e.g. for
struct S { struct T { char buf[5]; int x; } t; char buf[20]; } var;
With -D_FORTIFY_SOURCE=1,
strcpy (&var.t.buf[1], "abcdefg");
is not considered an overflow (object is whole VAR), while
with -D_FORTIFY_SOURCE=2
strcpy (&var.t.buf[1], "abcdefg");
will be considered a buffer overflow.
==================================================
 NOTE: In Ubuntu 8.10 and later versions, -D_FORTIFY_SOURCE=2
 is set by default, and is activated when -O is set to 2 or higher.
 This enables additional compile-time and run-time checks for several
 libc functions.  To disable, specify either -U_FORTIFY_SOURCE or
 -D_FORTIFY_SOURCE=0.
==================================================
It thus OK to keep the bug as RESOLVED

@rsc
Copy link
Contributor

rsc commented Dec 10, 2009

Comment 2:

Owner changed to r...@golang.org.

Status changed to Duplicate.

Merged into issue #393.

@gopherbot
Copy link
Author

Comment 3 by jeff.allen:

This is a different bug than 393, Russ.
The root cause of this one is that alloc() in lexbody goes into an infinite loop
allocating all memory in the machine if you ask it to allocate an object larger than
NHUNK (50000) bytes due to "while (nhunk < n) gethunk()".
A simple fix would be for it to refuse (return NULL), and let the caller deal with the
consequences. Another fix would be to use a two-tier allocation model, where big things
come direct from malloc and little things are managed by alloc().
I am still reading source to decide which of these might be best. Your suggestion?

@gopherbot
Copy link
Author

Comment 4 by jeff.allen:

It seems as all the users of alloc() do not free the memory (since the process is going
to exit soon anyway). So a simple change to a two-tier model works, because since there
is no free, the freer does not need to know which pool it came from.
Here's a proposed patch.
diff -r 9f5f89e6da5c src/cmd/cc/lexbody
--- a/src/cmd/cc/lexbody    Mon Oct 25 18:35:08 2010 +0200
+++ b/src/cmd/cc/lexbody    Mon Oct 25 19:24:12 2010 +0200
@@ -101,11 +101,15 @@
 {
    void *p;
 
+   /* for requests that are too big, don't use the hunk allocator */
+   if (n > NHUNK)
+       return malloc(n);
+
    while((uintptr)hunk & MAXALIGN) {
        hunk++;
        nhunk--;
    }
-   while(nhunk < n)
+   if (nhunk < n)
        gethunk();
    p = hunk;
    nhunk -= n;

@rsc
Copy link
Contributor

rsc commented Oct 25, 2010

Comment 5:

all the while loops should turn into calls to mal.
look in the linkers for examples.
(they all used to have those loops too.)
russ

@rsc
Copy link
Contributor

rsc commented Jan 19, 2011

Comment 6:

This issue was closed by revision 1558834.

Status changed to Fixed.

@golang golang locked and limited conversation to collaborators Jun 24, 2016
@rsc rsc removed their assignment Jun 22, 2022
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants