[Lldb-commits] [lldb] r235991 - Replace sprintf with snprintf to avoid a crash.

[Abid, Hafiz]

PRIx8 is defined as "x" in /usr/include/inttypes.h. This is Ubuntu 14.04.

#include <stdio.h>
 #include <string.h>
 #include <inttypes.h>
int main()
{
  char cUnescapedChar = 0xeb;
  const int old_size = sizeof("\\xXX");
  char strEscapedChar[old_size];
  ::sprintf(strEscapedChar, "\\x%02" PRIx8, cUnescapedChar);
  int new_size = strlen(strEscapedChar);
  printf("old:%d  new:%d\n", old_size, new_size);
  printf("%s\n", strEscapedChar);
  return 0;
}

gives this output on my system (with g++  4.8.2).
old:5  new:10
\xffffffeb
*** stack smashing detected ***: ./test terminated
Aborted (core dumped)

Casting the cUnescapedChar to unsigned char also fixes the crash.

Regards,
Abid

> -----Original Message-----
> From: Pavel Labath [mailto:labath at google.com]
> Sent: 28 April 2015 16:06
> To: Abid, Hafiz
> Cc: Ilia K; lldb-commits at cs.uiuc.edu
> Subject: Re: [Lldb-commits] [lldb] r235991 - Replace sprintf with snprintf to
> avoid a crash.
> 
> I find this surprising as well...
> 
> what is PRIx8 defined to on your system? Normally, it should be something
> like "hhx" as Ilia suggested. Here, "hh" should cast the value to char and "x"
> to unsigned, so there should be no possibility of overflow.
> 
> Unless I am misunderstanding something...
> 
> cheers,
> pl
> 
> 
> On 28 April 2015 at 15:55, Abid, Hafiz <Hafiz_Abid at mentor.com> wrote:
> > Well, it was happening :-(. It is \\x%02x” and cUnescapedChar is not
> > unsigned.
> >
> > So when it had value with MSB 1, then it overflew the buffer.
> >
> >
> >
> > There are other problems too with printing the 1-byte register.
> > Current code
> >
> > tries to print it as a character which is probably not the right
> > thing. But that
> >
> > is a separate fix.
> >
> >
> >
> > Regards,
> >
> > Abid
> >
> >
> >
> > From: Ilia K [mailto:ki.stfu at gmail.com]
> > Sent: 28 April 2015 15:26
> > To: Abid, Hafiz
> > Cc: lldb-commits at cs.uiuc.edu
> > Subject: Re: [Lldb-commits] [lldb] r235991 - Replace sprintf with
> > snprintf to avoid a crash.
> >
> >
> >
> > Hello Abid,
> >
> >
> >
> > I thought it never can happen because cUnescapedChar is less than 255
> > and "\\x%02hhu" prints something like \x12. Am I wrong?
> >
> >
> >
> > Thanks,
> >
> > Ilia
> >
> >
> >
> >
> >
> > On Tue, Apr 28, 2015 at 5:16 PM, Hafiz Abid Qadeer
> > <hafiz_abid at mentor.com>
> > wrote:
> >
> > Author: abidh
> > Date: Tue Apr 28 09:16:00 2015
> > New Revision: 235991
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=235991&view=rev
> > Log:
> > Replace sprintf with snprintf to avoid a crash.
> > During testing -data-list-register-values, I saw a crash here due to
> > buffer overflow.
> > This commit should fix the crash. There is still problem with printing
> > 1-byte register in some cases that I will fix separately.
> >
> > No regression on MI test cases.
> >
> >
> > Modified:
> >     lldb/trunk/tools/lldb-mi/MIUtilString.cpp
> >
> > Modified: lldb/trunk/tools/lldb-mi/MIUtilString.cpp
> > URL:
> > http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MIUtilStr
> > ing.cpp?rev=235991&r1=235990&r2=235991&view=diff
> >
> ================================================================
> ======
> > ========
> > --- lldb/trunk/tools/lldb-mi/MIUtilString.cpp (original)
> > +++ lldb/trunk/tools/lldb-mi/MIUtilString.cpp Tue Apr 28 09:16:00 2015
> > @@ -17,6 +17,7 @@
> >
> >  // In-house headers:
> >  #include "MIUtilString.h"
> > +#include "Platform.h"
> >
> >  //++
> > ----------------------------------------------------------------------
> > --------------  // Details: CMIUtilString constructor.
> > @@ -844,8 +845,9 @@ CMIUtilString::Escape(const bool vbEscap
> >                      strNew.push_back(cUnescapedChar);
> >                  else
> >                  {
> > -                    char strEscapedChar[sizeof("\\xXX")];
> > -                    ::sprintf(strEscapedChar, "\\x%02" PRIx8,
> > cUnescapedChar);
> > +                    const size_t size = sizeof("\\xXX");
> > +                    char strEscapedChar[size];
> > +                    ::snprintf(strEscapedChar, size, "\\x%02" PRIx8,
> > cUnescapedChar);
> >                      strNew.append(strEscapedChar);
> >                  }
> >                  break;
> >
> >
> > _______________________________________________
> > lldb-commits mailing list
> > lldb-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
> >
> >
> >
> >
> > _______________________________________________
> > lldb-commits mailing list
> > lldb-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
> >