Skip to content

Commit

Permalink
Implement actual fix
Browse files Browse the repository at this point in the history
  • Loading branch information
techraed committed Aug 25, 2024
1 parent 7f421a2 commit 9f41b85
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 56 deletions.
12 changes: 12 additions & 0 deletions core-errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,16 @@ pub enum MessageError {
#[display(fmt = "Outgoing messages bytes limit exceeded")]
OutgoingMessagesBytesLimitExceeded = 311,

/// The error occurs when a wrong offset of the input buffer (currently executing message payload)
/// is provided.
#[display(fmt = "Offset value for the input payload is out of it's size bounds")]
OutOfBoundsInputSliceOffset = 312,

/// The error occurs when a too big length value to form a slice (range) of the input buffer
/// (currently executing message payload) is provided.
#[display(fmt = "Too big length value is set to form a slice (range) of the input buffer")]
OutOfBoundsInputSliceLength = 313,

// TODO: remove after delay refactoring is done
/// An error occurs in attempt to charge gas for dispatch stash hold.
#[display(fmt = "Not enough gas to hold dispatch message")]
Expand Down Expand Up @@ -268,6 +278,8 @@ impl ExtError {
309 => Some(MessageError::DuplicateReplyDeposit.into()),
310 => Some(MessageError::IncorrectMessageForReplyDeposit.into()),
311 => Some(MessageError::OutgoingMessagesBytesLimitExceeded.into()),
312 => Some(MessageError::OutOfBoundsInputSliceOffset.into()),
313 => Some(MessageError::OutOfBoundsInputSliceLength.into()),
399 => Some(MessageError::InsufficientGasForDelayedSending.into()),
//
500 => Some(ReservationError::InvalidReservationId.into()),
Expand Down
83 changes: 66 additions & 17 deletions core-processor/src/ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -940,7 +940,10 @@ impl<LP: LazyPagesInterface> Externalities for Ext<LP> {
offset: u32,
len: u32,
) -> Result<(), Self::FallibleError> {
let range = self.context.message_context.check_input_range(offset, len);
let range = self
.context
.message_context
.check_input_range(offset, len)?;

self.with_changes(|mutator| {
mutator.charge_gas_if_enough(
Expand Down Expand Up @@ -1076,7 +1079,7 @@ impl<LP: LazyPagesInterface> Externalities for Ext<LP> {
let range = mutator
.context
.message_context
.check_input_range(offset, len);
.check_input_range(offset, len)?;
mutator.charge_gas_if_enough(
mutator
.context
Expand Down Expand Up @@ -1730,7 +1733,12 @@ mod tests {
.build(),
);

let data = HandlePacket::default();
let res = ext
.context
.message_context
.payload_mut()
.try_extend_from_slice(&[1, 2, 3, 4, 5, 6]);
assert!(res.is_ok());

let fake_handle = 0;

Expand All @@ -1742,20 +1750,37 @@ mod tests {

let handle = ext.send_init().expect("Outgoing limit is u32::MAX");

let res = ext
.context
.message_context
.payload_mut()
.try_extend_from_slice(&[1, 2, 3, 4, 5, 6]);
assert!(res.is_ok());

let res = ext.send_push_input(handle, 2, 3);
assert!(res.is_ok());

let res = ext.send_push_input(handle, 8, 10);
let res = ext.send_push_input(handle, 5, 1);
assert!(res.is_ok());

let msg = ext.send_commit(handle, data, 0);
// Len too big
let res = ext.send_push_input(handle, 0, 7);
assert_eq!(
res.unwrap_err(),
FallibleExtError::Core(FallibleExtErrorCore::Message(
MessageError::OutOfBoundsInputSliceLength
))
);
let res = ext.send_push_input(handle, 5, 2);
assert_eq!(
res.unwrap_err(),
FallibleExtError::Core(FallibleExtErrorCore::Message(
MessageError::OutOfBoundsInputSliceLength
))
);

// Too big offset
let res = ext.send_push_input(handle, 6, 0);
assert_eq!(
res.unwrap_err(),
FallibleExtError::Core(FallibleExtErrorCore::Message(
MessageError::OutOfBoundsInputSliceOffset
))
);

let msg = ext.send_commit(handle, HandlePacket::default(), 0);
assert!(msg.is_ok());

let res = ext.send_push_input(handle, 0, 1);
Expand All @@ -1774,7 +1799,7 @@ mod tests {
.map(|(dispatch, _, _)| dispatch)
.expect("Send commit was ok");

assert_eq!(dispatch.message().payload_bytes(), &[3, 4, 5]);
assert_eq!(dispatch.message().payload_bytes(), &[3, 4, 5, 6]);
}

#[test]
Expand Down Expand Up @@ -1889,10 +1914,34 @@ mod tests {

let res = ext.reply_push_input(2, 3);
assert!(res.is_ok());

let res = ext.reply_push_input(8, 10);
let res = ext.reply_push_input(5, 1);
assert!(res.is_ok());

// Len too big
let res = ext.reply_push_input(0, 7);
assert_eq!(
res.unwrap_err(),
FallibleExtError::Core(FallibleExtErrorCore::Message(
MessageError::OutOfBoundsInputSliceLength
))
);
let res = ext.reply_push_input(5, 2);
assert_eq!(
res.unwrap_err(),
FallibleExtError::Core(FallibleExtErrorCore::Message(
MessageError::OutOfBoundsInputSliceLength
))
);

// Too big offset
let res = ext.reply_push_input(6, 0);
assert_eq!(
res.unwrap_err(),
FallibleExtError::Core(FallibleExtErrorCore::Message(
MessageError::OutOfBoundsInputSliceOffset
))
);

let msg = ext.reply_commit(ReplyPacket::auto());
assert!(msg.is_ok());

Expand All @@ -1912,7 +1961,7 @@ mod tests {
.map(|(dispatch, _, _)| dispatch)
.expect("Send commit was ok");

assert_eq!(dispatch.message().payload_bytes(), &[3, 4, 5]);
assert_eq!(dispatch.message().payload_bytes(), &[3, 4, 5, 6]);
}

// TODO: fix me (issue #3881)
Expand Down
27 changes: 15 additions & 12 deletions core/src/message/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -428,24 +428,27 @@ impl MessageContext {
/// limits. Result `CheckedRange` instance is accepted by
/// `send_push_input`/`reply_push_input` and has the method `len`
/// allowing to charge gas before the calls.
pub fn check_input_range(&self, offset: u32, len: u32) -> CheckedRange {
pub fn check_input_range(&self, offset: u32, len: u32) -> Result<CheckedRange, Error> {
let input = self.current.payload_bytes();
let offset = offset as usize;
let len = len as usize;

// Check `offset` is not out of bounds.
if offset >= input.len() {
return CheckedRange {
offset: 0,
excluded_end: 0,
};
return Err(Error::OutOfBoundsInputSliceOffset);
}

CheckedRange {
offset,
excluded_end: if len == 0 {
offset
} else {
offset.saturating_add(len as usize).min(input.len())
},
// Check `len` for the current `offset` doesn't refer to the slice out of intput bounds.
let available_len = input.len() - offset;
if len > available_len {
return Err(Error::OutOfBoundsInputSliceLength);
}

Ok(CheckedRange {
offset,
// guaranteed to be `<= input.len()`, because of the check upper
excluded_end: offset.saturating_add(len),
})
}

/// Send reply message.
Expand Down
6 changes: 3 additions & 3 deletions examples/input-bug/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ mod wasm;

#[cfg(test)]
mod tests {
use alloc::vec::Vec;
use super::WASM_BINARY;
use gtest::{System, Program, constants};
use alloc::vec::Vec;
use gtest::{constants, Program, System};

#[test]
#[should_panic]
Expand Down Expand Up @@ -69,4 +69,4 @@ mod tests {
let sent_to_user_msg = log.pop().expect("checked");
assert!(!sent_to_user_msg.payload().is_empty())
}
}
}
5 changes: 4 additions & 1 deletion examples/input-bug/src/wasm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@
// You should have received a copy of the GNU General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.

use gstd::{prelude::*, msg::{self, MessageHandle}};
use gstd::{
msg::{self, MessageHandle},
prelude::*,
};

#[no_mangle]
extern "C" fn handle() {
Expand Down
19 changes: 10 additions & 9 deletions examples/proxy-relay/src/wasm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use gstd::msg::{self, MessageHandle};

static mut RELAY_CALL: Option<RelayCall> = None;

fn resend_push(resend_pushes: &[ResendPushData]) {
fn resend_push(resend_pushes: &[ResendPushData], size: usize) {
for data in resend_pushes {
let msg_handle = MessageHandle::init().expect("Failed to obtain new message handle");

Expand All @@ -35,7 +35,7 @@ fn resend_push(resend_pushes: &[ResendPushData]) {
match start.map(|s| s as usize) {
Some(s) => match end {
None => {
msg_handle.push_input(s..).expect("Push failed");
msg_handle.push_input(s..size).expect("Push failed");
}
Some((e, true)) => {
msg_handle.push_input(s..=e).expect("Push failed");
Expand All @@ -46,7 +46,7 @@ fn resend_push(resend_pushes: &[ResendPushData]) {
},
None => match end {
None => {
msg_handle.push_input(..).expect("Push failed");
msg_handle.push_input(..size).expect("Push failed");
}
Some((e, true)) => {
msg_handle.push_input(..=e).expect("Push failed");
Expand All @@ -67,26 +67,27 @@ fn resend_push(resend_pushes: &[ResendPushData]) {
extern "C" fn handle() {
use RelayCall::*;
let relay_call = unsafe { RELAY_CALL.as_ref().expect("Relay call is not initialized") };
let size = msg::size();

match relay_call {
Resend(d) => {
msg::send_input(*d, msg::value(), ..msg::size()).expect("Resend failed");
msg::send_input(*d, msg::value(), ..size).expect("Resend failed");
}
ResendWithGas(d, gas) => {
msg::send_input_with_gas(*d, *gas, msg::value(), ..).expect("Resend wgas failed");
msg::send_input_with_gas(*d, *gas, msg::value(), ..size).expect("Resend wgas failed");
}
ResendPush(data) => {
resend_push(data);
resend_push(data, size);
}
Rereply => {
msg::reply_input(msg::value(), 0..msg::size()).expect("Rereply failed");
msg::reply_input(msg::value(), 0..size).expect("Rereply failed");
}
RereplyPush => {
msg::reply_push_input(0..).expect("Push failed");
msg::reply_push_input(0..size).expect("Push failed");
msg::reply_commit(msg::value()).expect("Commit failed");
}
RereplyWithGas(gas) => {
msg::reply_input_with_gas(*gas, msg::value(), ..).expect("Rereply wgas failed");
msg::reply_input_with_gas(*gas, msg::value(), ..size).expect("Rereply wgas failed");
}
}
}
Expand Down
14 changes: 0 additions & 14 deletions pallets/gear/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13531,20 +13531,6 @@ fn relay_messages() {
payload: vec![],
},
),
(
RelayCall::ResendPush(vec![
// invalid range
ResendPushData {
destination: USER_3.into(),
start: Some(payload.len() as u32),
end: Some((0, false)),
},
]),
Expected {
user: USER_3,
payload: vec![],
},
),
];

for (call, expectation) in pairs {
Expand Down

0 comments on commit 9f41b85

Please sign in to comment.