#1676 buffer overflow in assembler

closed-fixed
assembler (26)
5
2014-09-05
2010-08-08
No

When I run the regression tests with the --debug flag the assembler crashes with a buffer overflow.

Processing uminus.c
*** buffer overflow detected ***: ../../bin/sdas6808 terminated
Processing uminus.c
*** buffer overflow detected ***: ../../bin/sdas8051 terminated
Processing uminus.c
*** buffer overflow detected ***: ../../bin/sdasz80 terminated

Tested with svn revision 5917.

Discussion

  • Maarten Brock

    Maarten Brock - 2010-08-15
    • milestone: --> fixed
    • assigned_to: nobody --> maartenbrock
    • status: open --> closed-fixed
     
  • Maarten Brock

    Maarten Brock - 2010-08-15

    Fixed by enlarging NCPS in the assembler in revision #5932.

     
  • Borut Ražem

    Borut Ražem - 2010-08-15

    Hi Maarten,

    this bug disclosed two probems:
    - the first one is that the diferrent identifiers match in first 80 characters. I haven't check the c standard yet, but I found this: http://publications.gbdirect.co.uk/c_book/chapter2/keywords_and_identifiers.html, so I don't know if this is really a sdcc bug, or we should somehow redesign the regression teting. But at this level I agree with your fix, even that I would be more satisfied if dsas / sdld would accept identifiers of arbitrary lenght (using malloc instead fixed lenght buffers).
    - the second one is: why it crashes? I took a quick look to the code and I didn't find the buffer overflows (neither did you fix them). Currently I can't chek it, since I'm on the WIN32 / cygwin machine which doesn't crash (the compiler doesn't produce run-time buffer overflow checks). Do you already know something more about it?

    Borut

     
  • Maarten Brock

    Maarten Brock - 2010-08-16

    The large identifier was a debug symbol which contains the filename. It was not an identifier in the source code.
    The buffer overflow check was performed by glibc sprintf in DefineCDB_Line() in asnoice.c.

    I agree that a malloc'ed buffer is better though it is also costlier esp. in performance.

     
  • Maarten Brock

    Maarten Brock - 2010-08-16

    On second thought, perhaps I should have fixed it by using PATH_MAX instead of NCPS as length for name in DefineCDB_Line(). After all it is a path it's printing here.

     
  • Borut Ražem

    Borut Ražem - 2010-08-16

    So my assumption about the problem was totally wrong :-(. I thought that sdld is crashing...

    I also think that revering NCPS back to 80 and increase the size on name[] array to PATH_MAX in function DefimeCDB_Line() is better solution.

    And use of snprintf() instead sprintf() would prevent the buffer overflow also in case of file name longer then PATH_MAX.

    Borut

     
  • Maarten Brock

    Maarten Brock - 2010-08-16

    Well my original post mentioned sdas crashing, not sdld. That should have been a give away ;-)

    I tried to use PATH_MAX directly now with NCPS back to 80, but as I kind of expected it just fails later. This time with multiple definition errors as the symbol is inserted in the lookup table.

    I'll look some further but for now I keep my fix.

     
  • Borut Ražem

    Borut Ražem - 2010-08-17

    > This time with multiple definition errors as the symbol is inserted in the lookup table.
    This is the one I saw and that's whi I serached in the wroh place.

    I've made a version which is using dbuf (dymanic buffer allocation), but I came to the same result as you: multiple definition errors.

    I commited the dbuf version but kept your change too. Be careful: I renamef asnoice.c to asdbg.c, to be more in sync with asxxxx.

    Borut

     
  • Borut Ražem

    Borut Ražem - 2010-08-17

    Now I fixed also the multiple definition errors problem: this one was in the sdld. I just defined NCPS as PATH_MAX in aslink.h. Proper solution would be support of arbitrary identifier length using dynamic buffers...

    P.S.: sorry for so many typos in my previous comment: it was just too early in the morning for may fingers...

    Borut

     
  • Maarten Brock

    Maarten Brock - 2010-08-18

    I also updated the MSVC project files yesterday with asdbg.c.

    I retested uminus.c with --debug and it failed with either NCPS=80 or NCPS=PATH_MAX in asxxxx.h. This turned out to be a dependency problem which I do not understand. If only asxxxx.h is touched my as8051 executable is not rebuilt. After a forced rebuild everything is fine.

    P.S.: I don't mind the typos but maybe you should try your august fingers instead of your may ones by now ;-)

    Maarten

     
  • Borut Ražem

    Borut Ražem - 2010-08-18

    I have the same problems with dependencies and I'm also using full rebuild to overcome them.

    > P.S.: I don't mind the typos but maybe you should try your august fingers instead of your may ones by now ;-)

    ROTFL :-)))))

    I'm actually using May fingers, the problem is that this May was 50 years ago :-(

    Borut

     
  • egan.fryazino

    egan.fryazino - 2014-09-05

    I have same error on pic16 sdcc-3.4.1 (2014 09 03)
    buffer overflow detected : /usr/local/bin/sdcc terminated
    ======= Backtrace: =========
    /lib64/libc.so.6[0x3431878b73]
    /lib64/libc.so.6(__fortify_fail+0x37)[0x34318fbe37]
    /lib64/libc.so.6[0x34318f9dc0]
    /lib64/libc.so.6[0x34318f9179]
    ...
    1 page simular text.

    my project:

     
    Attachments

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks