0

I'm hacking teeworlds' server code, however I'm experimenting a strange bug. I've already encountered something very similar, but I've found a not clean work around.

As I said in the post title, I've a segfault that appears only in release build. I located the line using printfs, because neither valgrind nor gdb show errors with the debug build.

this is the full code, the segfault occurs after have called this function :

  void CServer::SetClientName(int ClientID, const char *pName)
  {
     if(ClientID < 0 || ClientID >= MAX_CLIENTS || m_aClients[ClientID].m_State < CClient::STATE_READY)
        return;

     if(!pName)
        return;

     char * pDispName;
     char aCleanName[MAX_NAME_LENGTH];

     Console()->Print(IConsole::OUTPUT_LEVEL_STANDARD, "DEBUG", "TEST");
     // clean name
     int lastpos = 0;
     {
        int i = 0;
        const char *psrc = pName; 
        char *pdst = aCleanName;
        for(; *psrc != '\0' && *psrc<=' '; ++psrc);
        while(*psrc != '\0' && i<sizeof(aCleanName)-1)
        {
           if(*psrc <= ' ')
           {
              *pdst = ' ';
           }
           else
           {
              *pdst = *psrc;
              lastpos = i+1;
           }
           ++psrc;
           ++pdst;
           ++i;
        }
        aCleanName[lastpos] = '\0'; //Rtrim
     }
     Console()->Print(IConsole::OUTPUT_LEVEL_STANDARD, "DEBUG", "TEST2");
/////////////// These lines if not commented prevent the segfault
//     char aBuf[256];
//     str_format(aBuf, sizeof(aBuf), "CleanName : '%s, lastpos = %i'", aCleanName, lastpos);
     Console()->Print(IConsole::OUTPUT_LEVEL_STANDARD, "DEBUG", aCleanName);

     // set real name
     str_copy(m_aClients[ClientID].m_aName, aCleanName, sizeof(aCleanName));

     Console()->Print(IConsole::OUTPUT_LEVEL_STANDARD, "DEBUG", "TEST3");

     // DispName
     if(m_aClients[ClientID].m_aUserAcc[0] != '\0')
     {
        Console()->Print(IConsole::OUTPUT_LEVEL_STANDARD, "DEBUG", "AUTHED");
        pDispName = m_aClients[ClientID].m_aName;
     }
     else
     {
        Console()->Print(IConsole::OUTPUT_LEVEL_STANDARD, "DEBUG", "NOT AUTHED");
        str_percent_format(aCleanName, sizeof(aCleanName), g_Config.m_SvNotAuthedFormat, m_aClients[ClientID].m_aName);
        pDispName = aCleanName;
     }

     Console()->Print(IConsole::OUTPUT_LEVEL_STANDARD, "DEBUG", "TEST4");
     if(TrySetClientDispName(ClientID, pDispName))
     {
        Console()->Print(IConsole::OUTPUT_LEVEL_STANDARD, "DEBUG", "TRY FAILED");
        // auto rename
        int j = 0;
        for( ; j<MAX_NAME_LENGTH-3 && aCleanName[j]!='\0' ; ++j);
        Console()->Print(IConsole::OUTPUT_LEVEL_STANDARD, "DEBUG", "TEST5");
        for( ; j<MAX_NAME_LENGTH-3; ++j)
           aCleanName[j] = ' ';
        Console()->Print(IConsole::OUTPUT_LEVEL_STANDARD, "DEBUG", "TEST6");
        aCleanName[MAX_NAME_LENGTH-3] = '#';
        aCleanName[MAX_NAME_LENGTH-2] = 'a';
        aCleanName[MAX_NAME_LENGTH-1] = '\0';
        Console()->Print(IConsole::OUTPUT_LEVEL_STANDARD, "DEBUG", "TEST7");
        for(int i = 0; i<MAX_CLIENTS; ++i) //avoid infinite loop
        {
           Console()->Print(IConsole::OUTPUT_LEVEL_STANDARD, "DEBUG", "TEST9");
           if(TrySetClientDispName(ClientID, aCleanName) == 0)
              break;
           ++aCleanName[MAX_NAME_LENGTH-2];
        }
     }
     Console()->Print(IConsole::OUTPUT_LEVEL_STANDARD, "DEBUG", "TEST10");
  }

TEST10 is printed, but the statement fallowing the call is never

Note : TrySetClientDispName(ClientID, aCleanName) doesn't store pointers to aCleanName,

str_format is implementing as following :

  void str_format(char *buffer, int buffer_size, const char *format, ...)
  {
     va_list ap;
     va_start(ap, format);
     vsnprintf(buffer, buffer_size, format, ap);
     va_end(ap);
     buffer[buffer_size-1] = 0; /* assure null termination */
  }

I guess, it's a stack corruption, but where ? And then why does the str_format prevent the bug ?

EDIT : Maybe str_percent_format(aCleanName, sizeof(aCleanName), g_Config.m_SvNotAuthedFormat, m_aClients[ClientID].m_aName) write more in aCleanName than it can hold

hl037_
  • 3,520
  • 1
  • 27
  • 58
  • what is the value of `MAX_NAME_LENGTH`? You could be trying to write more into `aBuf` then it can hold. – NathanOliver Apr 14 '15 at 15:21
  • `MAX_NAME_LEGNTH` is 16. the segfault is not to with `aBuf` because it's when it is **NOT** present that there is segfault (actually, it's the `str_format` that prevent the segfault) – hl037_ Apr 14 '15 at 15:24
  • And what did valgrind show with the release build? – Useless Apr 14 '15 at 16:05
  • just a segfault (invalid read at) with the backtrace (no line numbers), an then something like "address 0c18 not mapped" – hl037_ Apr 14 '15 at 18:39

1 Answers1

0

That's it.

in such bugs, if a buffer has a strange behavior when used with another function, and a segfault appear after function end, it's because the buffer overflow corrupts the call stack.

by the way, that could do a great backdoor :D

hl037_
  • 3,520
  • 1
  • 27
  • 58
  • But why only in release and not in debug? – vesperto Feb 10 '22 at 09:36
  • 1
    Because in debug, memory is generally initialized to 0 and some protection may work. While in release, the compiler doesn't add the overflow of memory initialisation. (there are also some stack protection enable by default I think on debug, that involves some sentinel values on the stack. See https://stackoverflow.com/questions/1629685/when-and-how-to-use-gccs-stack-protection-feature – hl037_ Feb 11 '22 at 09:39