Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

MDEV-36245 - Long server_audit_file_path causes buffer overflow #3874

Open
wants to merge 1 commit into
base: 10.5
Choose a base branch
from

Conversation

tonychen2001
Copy link
Contributor

Description

Currently, the length of the server_audit_file_path system variable value is
not checked. This can cause a buffer overflow if given a long file path
specifying a directory as a memcpy() is performed to copy the entire file path into a
fixed size buffer, char alt_path_buffer[FN_REFLEN+1+DEFAULT_FILENAME_LEN];

We now add a check on the length of this value and reject the new value accordingly.

In file_logger:logger_open(), there is a check:

if (new_log.path_len+n_dig(rotations)+1 > FN_REFLEN)
// handle error

As n_dig(rotations) may return up to 3, this inherently limits the file path to
at most FN_REFLEN - 4 = 512 - 4 = 508 characters.

Release Notes

Fixed buffer overflow in server_audit plugin when specifying long directory paths for server_audit_file_path.

How can this PR be tested?

Create a directory of length > FN_REFLEN+1+DEFAULT_FILENAME_LEN = 512+1+16 = 529

$ realpath /quick-rebuilds/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | wc -c
580

Install audit plugin, enable logging, and set server_audit_file_path to the long directory path

MariaDB [(none)]> install plugin server_audit soname 'server_audit.so';
MariaDB [(none)]> set global server_audit_logging = ON;
MariaDB [(none)]> set global server_audit_file_path = '/quick-rebuilds/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa';

Before

*** buffer overflow detected ***: terminated
250306  6:36:10 [ERROR] ./build/sql/mariadbd got signal 6 ;
Sorry, we probably made a mistake, and this is a bug.
...
Thread pointer: 0x7f4ac8000c68
stack_bottom = 0x7f4b70410000 thread_stack 0x49000
addr2line: './build/sql/mariadbd': No such file
Printing to addr2line failed
./build/sql/mariadbd(my_print_stacktrace+0x2e)[0x564bb62f26ee]
./build/sql/mariadbd(handle_fatal_signal+0x19b)[0x564bb5ecc73b]
...
server_audit/server_audit.c:1133(start_logging)[0x7f4b8002d5ff]
server_audit/server_audit.c:2855(update_file_path)[0x7f4b8002da36]
addr2line: './build/sql/mariadbd': No such file
./build/sql/mariadbd(_ZN17sys_var_pluginvar13global_updateEP3THDP7set_var+0x8b)[0x564bb5cf2d4b]
./build/sql/mariadbd(_ZN7sys_var6updateEP3THDP7set_var+0xb5)[0x564bb5c40745]
./build/sql/mariadbd(_ZN7set_var6updateEP3THD+0x6c)[0x564bb5c41dac]
./build/sql/mariadbd(_Z17sql_set_variablesP3THDP4ListI12set_var_baseEb+0xa9)[0x564bb5c41a39]
...

After

server_audit: logging started to the file server_audit.log.
server_audit: server_audit_file_path can't exceed 508 characters.

MariaDB [(none)]> show warnings;
+---------+------+------------------------------------------------------+
| Level   | Code | Message                                              |
+---------+------+------------------------------------------------------+
| Warning |    1 | server_audit_file_path can't exceed 508 characters.  |
+---------+------+------------------------------------------------------+
1 row in set (0.000 sec)

MariaDB [(none)]> select @@server_audit_file_path;
+--------------------------+
| @@server_audit_file_path |
+--------------------------+
| server_audit.log         |
+--------------------------+
1 row in set (0.000 sec)

Basing the PR against the correct MariaDB version

  • This is a bug fix, and the PR is based against the earliest maintained branch in which the bug can be reproduced.

PR quality check

Copyright

All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc.

Currently, the length of this value is not checked and can cause a buffer
overflow if given a long file path specifying a directory.

In file_logger:logger_open(), there is a check:
```
  if (new_log.path_len+n_dig(rotations)+1 > FN_REFLEN)
    // handle error
```

As n_dig(rotations) may return up to 3, this inherently limits the file path to
FN_REFLEN - 4 characters.

All new code of the whole pull request, including one or several files that are
either new files or modified ones, are contributed under the BSD-new license. I
am contributing on behalf of my employer Amazon Web Services, Inc.
@svoj svoj added the External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements. label Mar 7, 2025
@svoj svoj changed the title Long server_audit_file_path causes buffer overflow MDEV-36245 - Long server_audit_file_path causes buffer overflow Mar 7, 2025
@svoj svoj requested a review from holyfoot March 7, 2025 20:35
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements.
Development

Successfully merging this pull request may close these issues.

3 participants