[bluez,v3] monitor: fix buffer overflow when terminal width > 255

Commit Message

Celeste Liu Sept. 17, 2024, 6:30 a.m. UTC

In current code, we create line buffer with size 256, which can contains
255 ASCII characters. But in modern system, terminal can have larger
width. It may cause buffer overflow in snprintf() text.

limits.h provides constant LINE_MAX.

    {LINE_MAX}
        Unless otherwise noted, the maximum length, in bytes, of a
        utility's input line (either standard input or another
        file), when the utility is described as processing text
        files. The length includes room for the trailing <newline>.
        Minimum Acceptable Value: {_POSIX2_LINE_MAX}

Signed-off-by: Celeste Liu <[e-mail]>
---
Changes in v3:
- Use constant LINE_MAX in limits.h instead dynamic heap allocation.
- Link to v2: https://lore.kernel.org/r/[e-mail]

Changes in v2:
- Add free() forgot in v1.
- Link to v1: https://patch.msgid.link/[e-mail]
---
 monitor/packet.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


---
base-commit: 41f943630d9a03c40e95057b2ac3d96470b9c71e
change-id: 20240914-fix-log-buffer-overflow-9aa5e61ee5b8

Best regards,

Comments

[e-mail] Sept. 17, 2024, 8:09 a.m. UTC | #1

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=890823

---Test result---

Test Summary:
CheckPatch                    PASS      0.28 seconds
GitLint                       FAIL      0.45 seconds
BuildEll                      PASS      24.54 seconds
BluezMake                     PASS      1727.17 seconds
MakeCheck                     PASS      13.74 seconds
MakeDistcheck                 PASS      181.34 seconds
CheckValgrind                 PASS      259.09 seconds
CheckSmatch                   WARNING   363.88 seconds
bluezmakeextell               PASS      120.05 seconds
IncrementalBuild              PASS      1423.05 seconds
ScanBuild                     PASS      992.04 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[bluez,v3] monitor: fix buffer overflow when terminal width > 255

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
20: B1 Line exceeds max length (100>80): "- Link to v2: https://lore.kernel.org/r/[e-mail]"
24: B1 Line exceeds max length (99>80): "- Link to v1: https://patch.msgid.link/[e-mail]"
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
monitor/packet.c: note: in included file:monitor/display.h:82:26: warning: Variable length array is used.monitor/packet.c:1868:26: warning: Variable length array is used.monitor/packet.c: note: in included file:monitor/bt.h:3606:52: warning: array of flexible structuresmonitor/bt.h:3594:40: warning: array of flexible structures


---
Regards,
Linux Bluetooth

[e-mail] Sept. 17, 2024, 2:20 p.m. UTC | #2

Hello:

This patch was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <[e-mail]>:

On Tue, 17 Sep 2024 14:30:46 +0800 you wrote:
> In current code, we create line buffer with size 256, which can contains
> 255 ASCII characters. But in modern system, terminal can have larger
> width. It may cause buffer overflow in snprintf() text.
> 
> limits.h provides constant LINE_MAX.
> 
>     {LINE_MAX}
>         Unless otherwise noted, the maximum length, in bytes, of a
>         utility's input line (either standard input or another
>         file), when the utility is described as processing text
>         files. The length includes room for the trailing <newline>.
>         Minimum Acceptable Value: {_POSIX2_LINE_MAX}
> 
> [...]

Here is the summary with links:
  - [bluez,v3] monitor: fix buffer overflow when terminal width > 255
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=2908491c7efe

You are awesome, thank you!

Patch

diff --git a/monitor/packet.c b/monitor/packet.c
index c2599fe6864ab44d657c121fcc3ceecc1ebc52a6..32a440bbea6888ab6321e973dbb23d9728864557 100644
--- a/monitor/packet.c
+++ b/monitor/packet.c
@@ -26,6 +26,7 @@ 
 #include <time.h>
 #include <sys/time.h>
 #include <sys/socket.h>
+#include <limits.h>
 
 #include "lib/bluetooth.h"
 #include "lib/uuid.h"
@@ -376,7 +377,7 @@  static void print_packet(struct timeval *tv, struct ucred *cred, char ident,
 					const char *text, const char *extra)
 {
 	int col = num_columns();
-	char line[256], ts_str[96], pid_str[140];
+	char line[LINE_MAX], ts_str[96], pid_str[140];
 	int n, ts_len = 0, ts_pos = 0, len = 0, pos = 0;
 	static size_t last_frame;