mono / gtk-sharp

Gtk# is a Mono/.NET binding to the cross platform Gtk+ GUI toolkit and the foundation of most GUI apps built with Mono
http://www.mono-project.com/GtkSharp
Other
426 stars 141 forks source link

Fix GLib.Log.Write() format string vulnerability via new glue #226

Closed canvon closed 6 years ago

canvon commented 6 years ago

(See https://gist.github.com/canvon/0fa8fcadd11ca8c72c9c79850f99b349 for a demonstration. It's not certain to me, though, that any code in the wild actually uses GLib.Log.Write().)

The third parameter of g_logv(), as used in GLib.Log.Write(), is actually not a message (as advertised in the prototype) but a format string.

The formatting done via String.Format() may leave format specifiers like %d unescaped, which can then lead to a crash / SIGSEGV while executing native code. Note that such format specifiers may get injected via something that then is supposed to get logged via the affected GLib.Log.Write().

As I'm told that marshalling to a variable arguments function is not supported, a new glue function, glibsharp_log_msg(), gets introduced (in a new glue file, glib/glue/log.c). Its only purpose is to replace passing the message as format string (which is wrong) with passing "%s" as format string with the message following as argument which is used as plain (non-format) string. This fixes the crash.

Therzok commented 6 years ago

Hey, I took the liberty of pushing a commit on top of your branch. This should simplify the code by quite a bit, by using CDecl specifics as to not create glue code for PInvoking here. Thanks for the PR, I'm testing it now then merging!

Therzok commented 6 years ago

Okay, this works! Thanks for the PR and pointing out the issue!