Skip to content

Commit 65d0f1a

Browse files
laanwjfanquake
andcommitted
devtools: Add security check for separate_code
Check that sections are appropriately separated in virtual memory, based on their (expected) permissions. This checks for missing -Wl,-z,separate-code and potentially other problems. Co-authored-by: fanquake <[email protected]>
1 parent 2e9e637 commit 65d0f1a

File tree

2 files changed

+99
-17
lines changed

2 files changed

+99
-17
lines changed

contrib/devtools/security-check.py

Lines changed: 92 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,25 +40,48 @@ def get_ELF_program_headers(executable):
4040
stdout = run_command([READELF_CMD, '-l', '-W', executable])
4141

4242
in_headers = False
43-
count = 0
4443
headers = []
4544
for line in stdout.splitlines():
4645
if line.startswith('Program Headers:'):
4746
in_headers = True
47+
count = 0
4848
if line == '':
4949
in_headers = False
5050
if in_headers:
5151
if count == 1: # header line
52-
ofs_typ = line.find('Type')
53-
ofs_offset = line.find('Offset')
54-
ofs_flags = line.find('Flg')
55-
ofs_align = line.find('Align')
56-
if ofs_typ == -1 or ofs_offset == -1 or ofs_flags == -1 or ofs_align == -1:
52+
header = [x.strip() for x in line.split()]
53+
ofs_typ = header.index('Type')
54+
ofs_flags = header.index('Flg')
55+
# assert readelf output is what we expect
56+
if ofs_typ == -1 or ofs_flags == -1:
5757
raise ValueError('Cannot parse elfread -lW output')
5858
elif count > 1:
59-
typ = line[ofs_typ:ofs_offset].rstrip()
60-
flags = line[ofs_flags:ofs_align].rstrip()
61-
headers.append((typ, flags))
59+
splitline = [x.strip() for x in line.split()]
60+
typ = splitline[ofs_typ]
61+
if not typ.startswith('[R'): # skip [Requesting ...]
62+
splitline = [x.strip() for x in line.split()]
63+
flags = splitline[ofs_flags]
64+
# check for 'R', ' E'
65+
if splitline[ofs_flags + 1] is 'E':
66+
flags += ' E'
67+
headers.append((typ, flags, []))
68+
count += 1
69+
70+
if line.startswith(' Section to Segment mapping:'):
71+
in_mapping = True
72+
count = 0
73+
if line == '':
74+
in_mapping = False
75+
if in_mapping:
76+
if count == 1: # header line
77+
ofs_segment = line.find('Segment')
78+
ofs_sections = line.find('Sections...')
79+
if ofs_segment == -1 or ofs_sections == -1:
80+
raise ValueError('Cannot parse elfread -lW output')
81+
elif count > 1:
82+
segment = int(line[ofs_segment:ofs_sections].strip())
83+
sections = line[ofs_sections:].strip().split()
84+
headers[segment][2].extend(sections)
6285
count += 1
6386
return headers
6487

@@ -68,7 +91,7 @@ def check_ELF_NX(executable) -> bool:
6891
'''
6992
have_wx = False
7093
have_gnu_stack = False
71-
for (typ, flags) in get_ELF_program_headers(executable):
94+
for (typ, flags, _) in get_ELF_program_headers(executable):
7295
if typ == 'GNU_STACK':
7396
have_gnu_stack = True
7497
if 'W' in flags and 'E' in flags: # section is both writable and executable
@@ -82,7 +105,7 @@ def check_ELF_RELRO(executable) -> bool:
82105
Dynamic section must have BIND_NOW flag
83106
'''
84107
have_gnu_relro = False
85-
for (typ, flags) in get_ELF_program_headers(executable):
108+
for (typ, flags, _) in get_ELF_program_headers(executable):
86109
# Note: not checking flags == 'R': here as linkers set the permission differently
87110
# This does not affect security: the permission flags of the GNU_RELRO program
88111
# header are ignored, the PT_LOAD header determines the effective permissions.
@@ -113,6 +136,62 @@ def check_ELF_Canary(executable) -> bool:
113136
ok = True
114137
return ok
115138

139+
def check_ELF_separate_code(executable):
140+
'''
141+
Check that sections are appropriately separated in virtual memory,
142+
based on their permissions. This checks for missing -Wl,-z,separate-code
143+
and potentially other problems.
144+
'''
145+
EXPECTED_FLAGS = {
146+
# Read + execute
147+
'.init': 'R E',
148+
'.plt': 'R E',
149+
'.plt.got': 'R E',
150+
'.plt.sec': 'R E',
151+
'.text': 'R E',
152+
'.fini': 'R E',
153+
# Read-only data
154+
'.interp': 'R',
155+
'.note.gnu.property': 'R',
156+
'.note.gnu.build-id': 'R',
157+
'.note.ABI-tag': 'R',
158+
'.gnu.hash': 'R',
159+
'.dynsym': 'R',
160+
'.dynstr': 'R',
161+
'.gnu.version': 'R',
162+
'.gnu.version_r': 'R',
163+
'.rela.dyn': 'R',
164+
'.rela.plt': 'R',
165+
'.rodata': 'R',
166+
'.eh_frame_hdr': 'R',
167+
'.eh_frame': 'R',
168+
'.qtmetadata': 'R',
169+
'.gcc_except_table': 'R',
170+
'.stapsdt.base': 'R',
171+
# Writable data
172+
'.init_array': 'RW',
173+
'.fini_array': 'RW',
174+
'.dynamic': 'RW',
175+
'.got': 'RW',
176+
'.data': 'RW',
177+
'.bss': 'RW',
178+
}
179+
# For all LOAD program headers get mapping to the list of sections,
180+
# and for each section, remember the flags of the associated program header.
181+
flags_per_section = {}
182+
for (typ, flags, sections) in get_ELF_program_headers(executable):
183+
if typ == 'LOAD':
184+
for section in sections:
185+
assert(section not in flags_per_section)
186+
flags_per_section[section] = flags
187+
# Spot-check ELF LOAD program header flags per section
188+
# If these sections exist, check them against the expected R/W/E flags
189+
for (section, flags) in flags_per_section.items():
190+
if section in EXPECTED_FLAGS:
191+
if EXPECTED_FLAGS[section] != flags:
192+
return False
193+
return True
194+
116195
def get_PE_dll_characteristics(executable) -> int:
117196
'''Get PE DllCharacteristics bits'''
118197
stdout = run_command([OBJDUMP_CMD, '-x', executable])
@@ -225,7 +304,8 @@ def check_MACHO_Canary(executable) -> bool:
225304
('PIE', check_ELF_PIE),
226305
('NX', check_ELF_NX),
227306
('RELRO', check_ELF_RELRO),
228-
('Canary', check_ELF_Canary)
307+
('Canary', check_ELF_Canary),
308+
('separate_code', check_ELF_separate_code),
229309
],
230310
'PE': [
231311
('DYNAMIC_BASE', check_PE_DYNAMIC_BASE),

contrib/devtools/test-security-check.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,17 @@ def test_ELF(self):
3131
cc = 'gcc'
3232
write_testcode(source)
3333

34-
self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-zexecstack','-fno-stack-protector','-Wl,-znorelro','-no-pie','-fno-PIE']),
34+
self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-zexecstack','-fno-stack-protector','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']),
3535
(1, executable+': failed PIE NX RELRO Canary'))
36-
self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fno-stack-protector','-Wl,-znorelro','-no-pie','-fno-PIE']),
36+
self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fno-stack-protector','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']),
3737
(1, executable+': failed PIE RELRO Canary'))
38-
self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fstack-protector-all','-Wl,-znorelro','-no-pie','-fno-PIE']),
38+
self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fstack-protector-all','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']),
3939
(1, executable+': failed PIE RELRO'))
40-
self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fstack-protector-all','-Wl,-znorelro','-pie','-fPIE']),
40+
self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fstack-protector-all','-Wl,-znorelro','-pie','-fPIE', '-Wl,-z,separate-code']),
4141
(1, executable+': failed RELRO'))
42-
self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fstack-protector-all','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE']),
42+
self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fstack-protector-all','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,noseparate-code']),
43+
(1, executable+': failed separate_code'))
44+
self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fstack-protector-all','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,separate-code']),
4345
(0, ''))
4446

4547
def test_PE(self):

0 commit comments

Comments
 (0)