Merge lp:~mc-return/compiz/compiz.merge-fix1101641-use-snprintf-instead-of-sprintf into lp:compiz/0.9.9

Proposed by MC Return
Status: Merged
Approved by: Timo Jyrinki
Approved revision: 3593
Merged at revision: 3588
Proposed branch: lp:~mc-return/compiz/compiz.merge-fix1101641-use-snprintf-instead-of-sprintf
Merge into: lp:compiz/0.9.9
Diff against target: 133 lines (+12/-12)
7 files modified
libdecoration/decoration.c (+1/-1)
plugins/composite/src/screen.cpp (+1/-1)
plugins/dbus/src/dbus.cpp (+2/-2)
plugins/loginout/src/loginout.cpp (+2/-2)
plugins/screenshot/src/screenshot.cpp (+1/-1)
plugins/water/src/water.cpp (+3/-3)
src/screen.cpp (+2/-2)
To merge this branch: bzr merge lp:~mc-return/compiz/compiz.merge-fix1101641-use-snprintf-instead-of-sprintf
Reviewer Review Type Date Requested Status
Sam Spilsbury Approve
PS Jenkins bot continuous-integration Pending
Review via email: mp+145228@code.launchpad.net

Commit message

Followed recommendations of Coverity and replaced sprintf () with snprintf ()
with respective buffer sizes.

(LP: #957587, #1101499, #1101565, #1101571, #1101641, #1101605)

Description of the change

As requested I am fixing all of those closely related SECURE_CODING issues
in just one MP, hope the bots can cope with that... ;)

To post a comment you must log in.
3593. By MC Return

Followed a recommendation of Coverity and replaced sprintf () with snprintf () with a buffer size of 128, as the definition says char buf[128]

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

This is all good as is.

Generally stuff like this:

44 + snprintf (path, 256, "%s/%s/%s/%s", COMPIZ_DBUS_ROOT_PATH,
45 plugin.c_str (), "options", o->name ().c_str ());

scares me because it can be truncated quite easily (leading to subtle problems). But, it isn't a problem with this review generally and we can fix it later.

review: Approve
Revision history for this message
MC Return (mc-return) wrote :

> This is all good as is.
>
> Generally stuff like this:
>
> 44 + snprintf (path, 256, "%s/%s/%s/%s", COMPIZ_DBUS_ROOT_PATH,
> 45 plugin.c_str (), "options", o->name ().c_str ());
>
> scares me because it can be truncated quite easily (leading to subtle
> problems). But, it isn't a problem with this review generally and we can fix
> it later.

TBH, that scares me too ;)

Thanx 4 the review :)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'libdecoration/decoration.c'
2--- libdecoration/decoration.c 2013-01-22 00:46:04 +0000
3+++ libdecoration/decoration.c 2013-01-28 20:55:24 +0000
4@@ -3268,7 +3268,7 @@
5 Atom dm_sn_atom;
6 char buf[128];
7
8- sprintf (buf, "_COMPIZ_DM_S%d", screen);
9+ snprintf (buf, 128, "_COMPIZ_DM_S%d", screen);
10 dm_sn_atom = XInternAtom (xdisplay, buf, 0);
11
12 if (xevent->xselectionclear.selection == dm_sn_atom)
13
14=== modified file 'plugins/composite/src/screen.cpp'
15--- plugins/composite/src/screen.cpp 2012-11-18 00:22:02 +0000
16+++ plugins/composite/src/screen.cpp 2013-01-28 20:55:24 +0000
17@@ -340,7 +340,7 @@
18 Window currentCmSnOwner;
19 char buf[128];
20
21- sprintf (buf, "_NET_WM_CM_S%d", screen->screenNum ());
22+ snprintf (buf, 128, "_NET_WM_CM_S%d", screen->screenNum ());
23 cmSnAtom = XInternAtom (dpy, buf, 0);
24
25 currentCmSnOwner = XGetSelectionOwner (dpy, cmSnAtom);
26
27=== modified file 'plugins/dbus/src/dbus.cpp'
28--- plugins/dbus/src/dbus.cpp 2012-08-03 10:12:25 +0000
29+++ plugins/dbus/src/dbus.cpp 2013-01-28 20:55:24 +0000
30@@ -282,7 +282,7 @@
31
32 for (s = d->screens; s; s = s->next)
33 {
34- sprintf (screenName, "screen%d", s->screenNum);
35+ snprintf (screenName, 256, "screen%d", s->screenNum);
36 dbusIntrospectAddNode (writer, screenName);
37 }
38 }
39@@ -1650,7 +1650,7 @@
40 if (!o)
41 return;
42
43- sprintf (path, "%s/%s/%s/%s", COMPIZ_DBUS_ROOT_PATH,
44+ snprintf (path, 256, "%s/%s/%s/%s", COMPIZ_DBUS_ROOT_PATH,
45 plugin.c_str (), "options", o->name ().c_str ());
46
47 signal = dbus_message_new_signal (path,
48
49=== modified file 'plugins/loginout/src/loginout.cpp'
50--- plugins/loginout/src/loginout.cpp 2012-07-30 09:53:16 +0000
51+++ plugins/loginout/src/loginout.cpp 2013-01-28 20:55:24 +0000
52@@ -245,7 +245,7 @@
53 * ourselves.
54 */
55
56- sprintf (buf, "WM_S%d", scr);
57+ snprintf (buf, 128, "WM_S%d", scr);
58
59 wmSnSelectionWindow = XInternAtom (screen->dpy (), buf, 0);
60
61@@ -267,7 +267,7 @@
62 char buf[128];
63 int scr = DefaultScreen (screen->dpy ());
64
65- sprintf (buf, "WM_S%d", scr);
66+ snprintf (buf, 128, "WM_S%d", scr);
67
68 XDeleteProperty (screen->dpy (), wmSnSelectionWindow,
69 kdeLogoutInfoAtom);
70
71=== modified file 'plugins/screenshot/src/screenshot.cpp'
72--- plugins/screenshot/src/screenshot.cpp 2012-10-20 10:01:27 +0000
73+++ plugins/screenshot/src/screenshot.cpp 2013-01-28 20:55:24 +0000
74@@ -209,7 +209,7 @@
75 if (n)
76 free (namelist);
77
78- sprintf (name, "screenshot%d.png", number);
79+ snprintf (name, 256, "screenshot%d.png", number);
80
81 CompString app (optionGetLaunchApp ());
82 CompString path (dir + "/" + name);
83
84=== modified file 'plugins/water/src/water.cpp'
85--- plugins/water/src/water.cpp 2012-12-10 03:28:47 +0000
86+++ plugins/water/src/water.cpp 2013-01-28 20:55:24 +0000
87@@ -264,13 +264,13 @@
88 set_water_vertices_fragment_shader);
89
90 if (target == GL_TEXTURE_2D)
91- sprintf (buf, update_water_vertices_fragment_shader.c_str (),
92+ snprintf (buf, 8192, update_water_vertices_fragment_shader.c_str (),
93 "2D", "2D",
94 1.0f / (float) texWidth, 1.0f / (float) texWidth,
95 1.0f / (float) texHeight, 1.0f / (float) texHeight,
96 "2D", "2D", "2D", "2D");
97 else
98- sprintf (buf, update_water_vertices_fragment_shader.c_str (),
99+ snprintf (buf, 8192, update_water_vertices_fragment_shader.c_str (),
100 "RECT", "RECT",
101 1.0f, 1.0f, 1.0f, 1.0f,
102 "RECT", "RECT", "RECT", "RECT");
103@@ -279,7 +279,7 @@
104 program[UPDATE] = new GLProgram (update_water_vertices_vertex_shader,
105 buffer);
106
107- sprintf (buf, paint_water_vertices_fragment_shader.c_str (),
108+ snprintf (buf, 8192, paint_water_vertices_fragment_shader.c_str (),
109 screen->width (), screen->height ());
110
111 buffer.assign (buf);
112
113=== modified file 'src/screen.cpp'
114--- src/screen.cpp 2013-01-11 08:03:50 +0000
115+++ src/screen.cpp 2013-01-28 20:55:24 +0000
116@@ -526,7 +526,7 @@
117 XGetErrorDatabaseText (dpy, "XlibMessage", "MajorCode", "%d", str, 128);
118 fprintf (stderr, str, e->request_code);
119
120- sprintf (str, "%d", e->request_code);
121+ snprintf (str, 128, "%d", e->request_code);
122 XGetErrorDatabaseText (dpy, "XRequest", str, "", str, 128);
123 if (strcmp (str, ""))
124 fprintf (stderr, " (%s)", str);
125@@ -4878,7 +4878,7 @@
126 /* Check for other window managers */
127
128 char buf[128];
129- sprintf (buf, "WM_S%d", DefaultScreen (dpy));
130+ snprintf (buf, 128, "WM_S%d", DefaultScreen (dpy));
131 wmSnAtom = XInternAtom (dpy, buf, 0);
132
133 Window currentWmSnOwner = XGetSelectionOwner (dpy, wmSnAtom);

Subscribers

People subscribed via source and target branches