From 5ca52e3af7e35cd0a7309d573595dcb78cce7fa7 Mon Sep 17 00:00:00 2001 From: Lim Sim Yee <137663782+simei2k@users.noreply.github.com> Date: Sat, 10 May 2025 22:33:18 +0800 Subject: [PATCH 1/3] Fix Integer Overflow Vulnerability in Buffer Write Method This pull request addresses a security vulnerability in the write() method that could lead to potential buffer overflow attacks through integer overflow in array bounds checking. The original implementation used a pattern vulnerable to integer overflow by checking array bounds with offset + length > b.length. This can be bypassed when both offset and length are large values that, when added, overflow to become a small value that passes the boundary check. This vulnerability was also identified in ReadyTalk/avian@0871979 and subsequently fixed. References: 1. ReadyTalk/avian@0871979 2. https://nvd.nist.gov/vuln/detail/cve-2020-9488 --- .../host/io/RobotFileOutputStream.java | 54 ++++++++++++++----- 1 file changed, 41 insertions(+), 13 deletions(-) diff --git a/robocode.host/src/main/java/net/sf/robocode/host/io/RobotFileOutputStream.java b/robocode.host/src/main/java/net/sf/robocode/host/io/RobotFileOutputStream.java index 9dc31c147..bac7e796b 100644 --- a/robocode.host/src/main/java/net/sf/robocode/host/io/RobotFileOutputStream.java +++ b/robocode.host/src/main/java/net/sf/robocode/host/io/RobotFileOutputStream.java @@ -1,4 +1,25 @@ -/* +@Override +public final void write(byte[] b, int off, int len) throws IOException { + // Check for null array + if (b == null) { + throw new NullPointerException(); + } + + // Comprehensive bounds checking to prevent integer overflow + if (off < 0 || len < 0 || len > b.length || off > b.length - len) { + throw new ArrayIndexOutOfBoundsException(); + } + + try { + fileSystemManager.checkQuota(len); + super.write(b, off, len); + } catch (IOException e) { + try { + close(); + } catch (IOException ignored) {} + throw e; + } +}/* * Copyright (c) 2001-2023 Mathew A. Nelson and Robocode contributors * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 @@ -46,18 +67,25 @@ public final void write(byte[] b) throws IOException { @Override public final void write(byte[] b, int off, int len) throws IOException { - if (len < 0) { - throw new IndexOutOfBoundsException(); - } - try { - fileSystemManager.checkQuota(len); - super.write(b, off, len); - } catch (IOException e) { - try { - close(); - } catch (IOException ignored) {} - throw e; - } + // Check for null array + if (b == null) { + throw new NullPointerException(); + } + + // Comprehensive bounds checking to prevent integer overflow + if (off < 0 || len < 0 || len > b.length || off > b.length - len) { + throw new ArrayIndexOutOfBoundsException(); + } + + try { + fileSystemManager.checkQuota(len); + super.write(b, off, len); + } catch (IOException e) { + try { + close(); + } catch (IOException ignored) {} + throw e; + } } @Override From 9f616173e5ed3b7b6c02c2b230b1014822bee363 Mon Sep 17 00:00:00 2001 From: "Flemming N. Larsen" Date: Tue, 13 May 2025 22:37:53 +0200 Subject: [PATCH 2/3] Removal of misplaced insertion of the same write() method before the header. Copy and paste error?! --- .../host/io/RobotFileOutputStream.java | 23 +------------------ 1 file changed, 1 insertion(+), 22 deletions(-) diff --git a/robocode.host/src/main/java/net/sf/robocode/host/io/RobotFileOutputStream.java b/robocode.host/src/main/java/net/sf/robocode/host/io/RobotFileOutputStream.java index bac7e796b..f08cd19a6 100644 --- a/robocode.host/src/main/java/net/sf/robocode/host/io/RobotFileOutputStream.java +++ b/robocode.host/src/main/java/net/sf/robocode/host/io/RobotFileOutputStream.java @@ -1,25 +1,4 @@ -@Override -public final void write(byte[] b, int off, int len) throws IOException { - // Check for null array - if (b == null) { - throw new NullPointerException(); - } - - // Comprehensive bounds checking to prevent integer overflow - if (off < 0 || len < 0 || len > b.length || off > b.length - len) { - throw new ArrayIndexOutOfBoundsException(); - } - - try { - fileSystemManager.checkQuota(len); - super.write(b, off, len); - } catch (IOException e) { - try { - close(); - } catch (IOException ignored) {} - throw e; - } -}/* +/* * Copyright (c) 2001-2023 Mathew A. Nelson and Robocode contributors * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 From 9787e2cc90942d94ae341cf5562e42495443084b Mon Sep 17 00:00:00 2001 From: "Flemming N. Larsen" Date: Tue, 13 May 2025 22:38:35 +0200 Subject: [PATCH 3/3] Formatted write() method --- .../host/io/RobotFileOutputStream.java | 39 ++++++++++--------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/robocode.host/src/main/java/net/sf/robocode/host/io/RobotFileOutputStream.java b/robocode.host/src/main/java/net/sf/robocode/host/io/RobotFileOutputStream.java index f08cd19a6..3b80ee055 100644 --- a/robocode.host/src/main/java/net/sf/robocode/host/io/RobotFileOutputStream.java +++ b/robocode.host/src/main/java/net/sf/robocode/host/io/RobotFileOutputStream.java @@ -46,25 +46,26 @@ public final void write(byte[] b) throws IOException { @Override public final void write(byte[] b, int off, int len) throws IOException { - // Check for null array - if (b == null) { - throw new NullPointerException(); - } - - // Comprehensive bounds checking to prevent integer overflow - if (off < 0 || len < 0 || len > b.length || off > b.length - len) { - throw new ArrayIndexOutOfBoundsException(); - } - - try { - fileSystemManager.checkQuota(len); - super.write(b, off, len); - } catch (IOException e) { - try { - close(); - } catch (IOException ignored) {} - throw e; - } + // Sanity check + if (b == null) { + throw new NullPointerException(); + } + + // Comprehensive bounds checking to prevent integer overflow + if (off < 0 || len < 0 || len > b.length || off > b.length - len) { + throw new ArrayIndexOutOfBoundsException(); + } + + try { + fileSystemManager.checkQuota(len); + super.write(b, off, len); + } catch (IOException e) { + try { + close(); + } catch (IOException ignore) { + } + throw e; + } } @Override