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

[Pavel Labath]

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/MIUtilString.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
>