Interger Overflow in `lib::write` leads to incorrect tracking of the total amount of data written

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-03-phala-network/blob/a01ffbe992560d8d0f17deadfb9b9a2bed38377e/phala-blockchain/crates/pink/chain-extension/src/lib.rs#L410-L411 https://github.com/code-423n4/2024-03-phala-network/blob/a01ffbe992560d8d0f17deadfb9b9a2bed38377e/phala-blockchain/crates/pink/chain-extension/src/lib.rs#L402-L403

Vulnerability details

Impact

write method could potentially suffer from an integer overflow when self.written + wlen exceeds the maximum value that can be stored in the LimitedWriter struct size.

Proof of Concept

The LimitedWriter struct is designed to limit the amount of data written to the underlying writer. It keeps track of the amount of data written so far in the written field. However, when the amount of data written is very large, adding wlen to self.written could cause an integer overflow, leading to incorrect tracking of the total amount of data written.

Problem occurs when the amount of data written is so large that adding the length of the new data (wlen) to self.written exceeds the maximum value that can be stored in a usize.

When self.written + wlen exceeds this maximum value, then the value wraps around to zero and starts counting up again. This results in a problem because self.written is no longer accurately tracking the total amount of data written. Instead, it's effectively reset to a much smaller value.

impl<W: std::io::Write> std::io::Write for LimitedWriter<W> {
    fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
        if self.written + buf.len() > self.limit {
            return Err(std::io::Error::new(
                std::io::ErrorKind::Other,
                "Buffer limit exceeded",
            ));
        }
        let wlen = self.writer.write(buf)?;
@>      self.written += wlen;
        Ok(wlen)
    }

    fn flush(&mut self) -> std::io::Result<()> {
        self.writer.flush()
    }
}

Tools Used

Manual

Recommended Mitigation Steps

checked_add method could be used, which returns None if the addition would cause an overflow. Here's the mitigated code

impl<W: std::io::Write> std::io::Write for LimitedWriter<W> {
    fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
        if self.written + buf.len() > self.limit {
            return Err(std::io::Error::new(
                std::io::ErrorKind::Other,
                "Buffer limit exceeded",
            ));
        }
        let wlen = self.writer.write(buf)?;
-       self.written += wlen;
+       match self.written.checked_add(wlen) {
+           Some(sum) => self.written = sum,
+           None => return Err(std::io::Error::new(
+               std::io::ErrorKind::Other,
+               "Integer overflow occurred",
+           )),
+      };   

        Ok(wlen)
    }

    fn flush(&mut self) -> std::io::Result<()> {
        self.writer.flush()
    }
}

Assessed type

Under/Overflow

[c4-pre-sort]

141345 marked the issue as insufficient quality report

[141345]

seems invalid there is check above

[c4-judge]

OpenCoreCH marked the issue as unsatisfactory: Invalid