Skip to content

Commit

Permalink
Code reviews
Browse files Browse the repository at this point in the history
- Use fstat() and fchmod() but not stat() and chmod() to
  fix race conditions
- Avoid to use after free
- Fix dereference of IBusComposeTable->priv
- Fix to divide by zero
  • Loading branch information
fujiwarat committed Jul 26, 2021
1 parent 4259f16 commit 17ae266
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 10 deletions.
23 changes: 18 additions & 5 deletions src/ibusbus.c
Expand Up @@ -21,13 +21,14 @@
* USA
*/

#include "ibusbus.h"
#include <errno.h>
#include <fcntl.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>
#include <glib/gstdio.h>
#include <gio/gio.h>
#include "ibusbus.h"
#include "ibusmarshalers.h"
#include "ibusinternal.h"
#include "ibusshare.h"
Expand Down Expand Up @@ -537,7 +538,8 @@ static void
ibus_bus_init (IBusBus *bus)
{
struct stat buf;
gchar *path;
char *path;
int fd;

bus->priv = IBUS_BUS_GET_PRIVATE (bus);

Expand All @@ -562,20 +564,31 @@ ibus_bus_init (IBusBus *bus)
return;
}

if (stat (path, &buf) == 0) {
errno = 0;
if ((fd = open (path, O_RDONLY | O_DIRECTORY, S_IRWXU)) == -1) {
g_warning ("open %s failed: %s", path, g_strerror (errno));
g_free (path);
return;
}
/* TOCTOU: Use fstat() and fchmod() but not stat() and chmod().
* because it can cause a time-of-check, time-of-use race condition.
*/
if (fstat (fd, &buf) == 0) {
if (buf.st_uid != getuid ()) {
g_warning ("The owner of %s is not %s!",
path, ibus_get_user_name ());
close (fd);
g_free (path);
return;
}
if (buf.st_mode != (S_IFDIR | S_IRWXU)) {
errno = 0;
if (g_chmod (path, 0700))
g_warning ("chmod failed: %s", errno ? g_strerror (errno) : "");
if (fchmod (fd, S_IRWXU))
g_warning ("chmod failed: %s", g_strerror (errno));
}
}

close (fd);
g_free (path);
}

Expand Down
8 changes: 6 additions & 2 deletions src/ibuscomposetable.c
Expand Up @@ -406,7 +406,7 @@ get_en_compose_file (void)
path = g_build_filename (X11_DATADIR, *sys_lang, "Compose", NULL);
if (g_file_test (path, G_FILE_TEST_EXISTS))
break;
g_free (path);
g_clear_pointer (&path, g_free);
}
return path;
}
Expand Down Expand Up @@ -975,7 +975,10 @@ ibus_compose_table_deserialize (const char *contents,
goto out_load_cache;
}
if (data_length) {
retval->priv = g_new0 (IBusComposeTablePrivate, 1);
if (!(retval->priv = g_new0 (IBusComposeTablePrivate, 1))) {
g_warning ("Failed g_new");
goto out_load_cache;
}
retval->priv->data_first = g_new (guint16, data_length);
memcpy (retval->priv->data_first,
data_32bit_first, data_length * sizeof (guint16));
Expand Down Expand Up @@ -1565,6 +1568,7 @@ ibus_compose_table_compact_check (const IBusComposeTableCompactEx
row_stride = i + 2;

if (seq_index[i + 1] - seq_index[i] > 0) {
g_assert (row_stride);
seq = bsearch (compose_buffer + 1,
table->data + seq_index[i],
(seq_index[i + 1] - seq_index[i]) / row_stride,
Expand Down
6 changes: 3 additions & 3 deletions src/tests/ibus-desktop-testing-runner.in
Expand Up @@ -220,7 +220,7 @@ init_gnome()
# G_MESSAGES_DEBUG=all or G_MESSAGES_DEBUG=GLib-GIO-DEBUG would append
# debug messages to gsettings output and could not get the result correctly.
backup_G_MESSAGES_DEBUG="$G_MESSAGES_DEBUG"
export -n G_MESSAGES_DEBUG=''
unset G_MESSAGES_DEBUG
# Disable Tour dialog to get focus
V=`gsettings get org.gnome.shell welcome-dialog-last-shown-version`
if [ x"$V" = x"''" ] ; then
Expand All @@ -231,8 +231,8 @@ init_gnome()
NO_SYS_DIR=/usr/share/gnome-shell/extensions/no-overview@fthx
NO_USER_DIR=$HOME/.local/share/gnome-shell/extensions/no-overview@fthx
if [ ! -d $NO_SYS_DIR ] && [ ! -d $NO_USER_DIR ] ; then
mkdir -p `dirname $NO_USER_DIR`
cp -R no-overview@fthx `dirname $NO_USER_DIR`
mkdir -p "`dirname $NO_USER_DIR`"
cp -R "no-overview@fthx" "`dirname $NO_USER_DIR`"
fi
V=`gsettings get org.gnome.shell disable-user-extensions`
if [ x"$V" = x"true" ] ; then
Expand Down

0 comments on commit 17ae266

Please sign in to comment.