Merge lp:~ted/lightdm-remote-session-freerdp/lp1039636 into lp:lightdm-remote-session-freerdp

Proposed by Ted Gould
Status: Merged
Approved by: Albert Astals Cid
Approved revision: 15
Merged at revision: 11
Proposed branch: lp:~ted/lightdm-remote-session-freerdp/lp1039636
Merge into: lp:lightdm-remote-session-freerdp
Diff against target: 50 lines (+16/-3)
2 files modified
Makefile.am (+5/-0)
socket-sucker.c (+11/-3)
To merge this branch: bzr merge lp:~ted/lightdm-remote-session-freerdp/lp1039636
Reviewer Review Type Date Requested Status
Albert Astals Cid (community) Approve
jenkins (community) continuous-integration Approve
Review via email: mp+121951@code.launchpad.net

This proposal supersedes a proposal from 2012-08-29.

Commit message

Checking return values and enabling PIE to increase security

Description of the change

Fixes the issues found by the security team during their audit of the package. Better checking of return values and also building with PIE.

To post a comment you must log in.
Revision history for this message
jenkins (martin-mrazik+qa) wrote :
review: Approve (continuous-integration)
15. By Ted Gould

Check to ensure we wrote something

Revision history for this message
jenkins (martin-mrazik+qa) wrote :
review: Approve (continuous-integration)
Revision history for this message
Albert Astals Cid (aacid) wrote :

Works

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Makefile.am'
2--- Makefile.am 2012-08-21 20:08:02 +0000
3+++ Makefile.am 2012-08-29 22:00:25 +0000
4@@ -22,6 +22,11 @@
5 socket-sucker
6 socket_sucker_SOURCES = \
7 socket-sucker.c
8+socket_sucker_CFLAGS = \
9+ -Wall -Werror \
10+ -fPIE
11+socket_sucker_LDFLAGS = \
12+ -pie
13
14 EXTRA_DIST = \
15 $(pam_session_DATA) \
16
17=== modified file 'socket-sucker.c'
18--- socket-sucker.c 2012-08-21 19:50:28 +0000
19+++ socket-sucker.c 2012-08-29 22:00:25 +0000
20@@ -41,7 +41,12 @@
21 }
22
23 serv_addr.sun_family = AF_UNIX;
24- snprintf(serv_addr.sun_path, sizeof(serv_addr.sun_path), "%s/%s", home, ".freerdp-socket");
25+
26+ int printsize = snprintf(serv_addr.sun_path, sizeof(serv_addr.sun_path) - 1, "%s/%s", home, ".freerdp-socket");
27+ if (printsize > sizeof(serv_addr.sun_path) - 1 || printsize < 0) {
28+ return -1;
29+ }
30+
31 servlen = strlen(serv_addr.sun_path) + sizeof(serv_addr.sun_family);
32
33 if ((socket_fd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
34@@ -57,11 +62,14 @@
35 int out = 0;
36
37 in = read(socket_fd, buffer, BUFFER_SIZE);
38- out = write(1, buffer, in);
39+
40+ if (in > 0) {
41+ out = write(1, buffer, in);
42+ }
43
44 close(socket_fd);
45
46- if (in == 0) {
47+ if (in > 0 && out > 0) {
48 return 0;
49 } else {
50 return -1;

Subscribers

People subscribed via source and target branches

to all changes: