Opened 6 years ago

Last modified 4 weeks ago

#14961 assigned bug

Audit all syscalls for permissions and access checks

Reported by: waddlesplash Owned by: waddlesplash
Priority: high Milestone: R1/beta6
Component: System/Kernel Version: R1/Development
Keywords: security Cc:
Blocked By: #15356 Blocking:
Platform: All

Description (last modified by waddlesplash)

Here's a full list of source files containing syscall implementations:

  • OK src/system/kernel/arch/arm/arch_atomic32.cpp
  • OK src/system/kernel/arch/arm/arch_atomic64.cpp
  • OK src/system/kernel/arch/m68k/arch_atomic.cpp
  • OK src/system/kernel/arch/x86/arch_system_info.cpp
  • OK src/system/kernel/arch/x86/syscalls_compat.cpp
  • OK src/system/kernel/debug/debug.cpp
  • OK src/system/kernel/debug/frame_buffer_console.cpp
  • OK src/system/kernel/debug/safemode_settings.cpp
  • OK src/system/kernel/debug/system_profiler.cpp
  • OK src/system/kernel/debug/tracing.cpp
  • OK src/system/kernel/debug/user_debugger.cpp
  • NEEDSWORK src/system/kernel/disk_device_manager/ddm_userland_interface.cpp
  • OK src/system/kernel/UserTimer.cpp
  • OK src/system/kernel/cpu.cpp
  • OK src/system/kernel/elf.cpp
  • OK src/system/kernel/fs/fd.cpp
  • NEEDSWORK src/system/kernel/fs/node_monitor.cpp
  • OK src/system/kernel/fs/socket.cpp
  • OK src/system/kernel/fs/vfs.cpp (except #15701)
  • src/system/kernel/image.cpp
  • src/system/kernel/locks/user_mutex.cpp
  • src/system/kernel/messaging/MessagingService.cpp
  • src/system/kernel/port.cpp
  • src/system/kernel/posix/realtime_sem.cpp
  • src/system/kernel/posix/xsi_message_queue.cpp
  • src/system/kernel/posix/xsi_semaphore.cpp
  • src/system/kernel/real_time_clock.cpp
  • src/system/kernel/scheduler/scheduler.cpp
  • src/system/kernel/scheduler/scheduling_analysis.cpp
  • src/system/kernel/sem.cpp
  • src/system/kernel/shutdown.cpp
  • src/system/kernel/signal.cpp
  • src/system/kernel/syscalls.cpp
  • src/system/kernel/system_info.cpp
  • src/system/kernel/team.cpp
  • src/system/kernel/thread.cpp
  • src/system/kernel/usergroup.cpp
  • src/system/kernel/vm/vm.cpp
  • src/system/kernel/wait_for_objects.cpp

Each and every one of these needs to be audited, namely:

  • All passed pointers are checked against IS_USER_ADDRESS (thanks to SMAP, largely already done)
  • All objects (e.g. FDs, areas, semaphores) manipulated by syscalls are checked that the calling team has access to manipulate them
  • Whatever other things I think of adding here...

Change History (16)

comment:1 by waddlesplash, 6 years ago

Description: modified (diff)

Did the first pass on the first few files.

safemode_settings does not do any access checks; any application is allowed to read (not write, that can't be done via syscall) safemode settings. I think that's OK? Otherwise the file is fine.

system_profiler also does not seem to do permissions checks. I think that we expose quite a lot of information in that, so it should probably only allow root to run the profiler?

comment:2 by waddlesplash, 6 years ago

Just for the record: I already added thread syscall permissions checks in hrev52825~1, and the first level of VM area access checks in hrev52546~2.

comment:3 by waddlesplash, 5 years ago

Description: modified (diff)

A bunch more files done in hrev53233.

comment:4 by waddlesplash, 5 years ago

Description: modified (diff)

kernel/fs/fd done in hrev53481.

comment:5 by waddlesplash, 5 years ago

Description: modified (diff)

comment:6 by waddlesplash, 5 years ago

Blocked By: 15356 added

comment:7 by kallisti5, 5 years ago

This one seems important, but is it a blocker for R1 Beta2? Seems more critical (something that could be fixed post-release in the branch)

comment:8 by pulkomandy, 5 years ago

Priority: blockerhigh

Yes, this does not deserve to be a blocker.

comment:9 by waddlesplash, 5 years ago

Description: modified (diff)

vfs.cpp done in hrev53864, with one caveat.

comment:10 by nielx, 5 years ago

Milestone: R1/beta2R1/beta3

Moving to R1/beta3 after discussion on IRC. This needs more time to simmer after changes have been made.

comment:11 by waddlesplash, 4 years ago

Permissions checks are missing in _user_unblock_thread.

comment:12 by pulkomandy, 3 years ago

Milestone: R1/beta3R1/beta4

Move unsolved issues to beta 4 since there is no chance of fixing them now.

comment:13 by waddlesplash, 2 years ago

Milestone: R1/beta4R1/beta5

Kicking the can down the road again on this one.

comment:14 by nephele, 7 months ago

Milestone: R1/beta5R1/beta6

Let's get beta5 out, this was already not finished in beta4, so nothing lost :)

comment:15 by waddlesplash, 4 weeks ago

The rootfs has some permissions checks, but I think it's missing some (e.g. create_special_node; but this may be tricky since I think FIFOs/pipes are placed on the rootfs by default.)

Note: See TracTickets for help on using tickets.