Skip to content

security: check if shared memory belongs to current user and is only …#43

Open
spaceone wants to merge 1 commit into
luizalabs:mainfrom
spaceone:security-information-disclosure
Open

security: check if shared memory belongs to current user and is only …#43
spaceone wants to merge 1 commit into
luizalabs:mainfrom
spaceone:security-information-disclosure

Conversation

@spaceone

@spaceone spaceone commented Nov 3, 2021

Copy link
Copy Markdown
Contributor

…read/writeable for them

Fixes #33

@spaceone spaceone force-pushed the security-information-disclosure branch 2 times, most recently from 2b2bcd8 to 2d50920 Compare November 3, 2021 14:20
…read/writeable for them

Prevents
1. information disclosure
2. unpickling of untrusted pickle files resulting in code execution
vulnerabilities

Execute as user `nobody`:
```
$ python3
>>> with open('/dev/shm/sm_foo', 'wb') as fd:
...  fd.write(b'\x80\x03csubprocess\ncall\nq\x00X\n\x00\x00\x00/bin/touchq\x01X\x0b\x00\x00\x00/tmp/hackedq\x02\x86q\x03\x85q\x04Rq\x05.')
...
66
$ ls -l '/dev/shm/sm_foo'
-rw-r--r-- 1 nobody nogroup 66 Okt 21 18:42 /dev/shm/sm_foo
```

Then execute a new process as any user (e.g. root):

```
$ python3
>>> import shared_memory_dict
>>> f = shared_memory_dict.SharedMemoryDict('foo', 500)
>>> f
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/fbest/git/shared-memory-dict/shared_memory_dict/dict.py", line 115, in __repr__
    return repr(self._read_memory())
  File "/home/fbest/git/shared-memory-dict/shared_memory_dict/dict.py", line 169, in _read_memory
    db = {key: self._unmap_value(key, value) for key, value in db.items()}
AttributeError: 'int' object has no attribute 'items'

$ ls -l /tmp/hacked
-rw-r--r-- 1 root root 0 Okt 21 18:45 /tmp/hacked
```

The command /bin/touch /tmp/hacked has been executed as root.

Fixes luizalabs#33
@spaceone spaceone force-pushed the security-information-disclosure branch from 2d50920 to 35ae4d0 Compare November 3, 2021 14:29
@mbwmbw1337

Copy link
Copy Markdown

Any updates on merging this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

security considerations

2 participants