Skip to content

Commit 3cf46a4

Browse files
authored
refactor(pyth-sdk-solana): refactor attribute iterators (#131)
* refactor(pyth-sdk-solana): refactor attribute iterators There are some assumptions outside this SDK on the product attributes and the `size` field in particular that guarantee that the accesses are safe. This commit refactors it to add extra levels of safety to not rely on those assumptions. * fix: update CI to use anza endpoint for solana binary * fix: add another test for iterating over attributes
1 parent 8c56e4f commit 3cf46a4

File tree

7 files changed

+89
-14
lines changed

7 files changed

+89
-14
lines changed

.github/workflows/check-formatting.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,4 +16,4 @@ jobs:
1616
profile: minimal
1717
toolchain: nightly
1818
components: rustfmt
19-
- uses: pre-commit/action@v2.0.3
19+
- uses: pre-commit/action@v3.0.0

.github/workflows/pyth-sdk-example-anchor-contract.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ jobs:
2121
run: sudo apt-get update && sudo apt-get install libudev-dev pkg-config build-essential protobuf-compiler
2222
- name: Install solana binaries
2323
run: |
24-
sh -c "$(curl -sSfL https://release.solana.com/v1.18.21/install)"
24+
sh -c "$(curl -sSfL https://release.anza.xyz/v1.18.21/install)"
2525
echo "/home/runner/.local/share/solana/install/active_release/bin" >> $GITHUB_PATH
2626
- name: Install anchor binaries
2727
run: |

.github/workflows/pyth-sdk-example-solana-contract.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ jobs:
2121
run: sudo apt-get update && sudo apt-get install libudev-dev protobuf-compiler
2222
- name: Install solana binaries
2323
run: |
24-
sh -c "$(curl -sSfL https://release.solana.com/v1.18.21/install)"
24+
sh -c "$(curl -sSfL https://release.anza.xyz/v1.18.21/install)"
2525
echo "/home/runner/.local/share/solana/install/active_release/bin" >> $GITHUB_PATH
2626
- name: Build
2727
run: scripts/build.sh

.github/workflows/pyth-sdk-solana.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ jobs:
3434
run: sudo apt-get update && sudo apt-get install libudev-dev protobuf-compiler
3535
- name: Install Solana Binaries
3636
run: |
37-
sh -c "$(curl -sSfL https://release.solana.com/v1.18.21/install)"
37+
sh -c "$(curl -sSfL https://release.anza.xyz/v1.18.21/install)"
3838
echo "/home/runner/.local/share/solana/install/active_release/bin" >> $GITHUB_PATH
3939
- name: Build
4040
run: cargo build --verbose

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pyth-sdk-solana/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "pyth-sdk-solana"
3-
version = "0.10.4"
3+
version = "0.10.5"
44
authors = ["Pyth Data Foundation"]
55
workspace = "../"
66
edition = "2018"

pyth-sdk-solana/src/state.rs

Lines changed: 83 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use pyth_sdk::{
1818
};
1919
use solana_program::clock::Clock;
2020
use solana_program::pubkey::Pubkey;
21+
use std::cmp::min;
2122
use std::mem::size_of;
2223

2324
pub use pyth_sdk::{
@@ -192,7 +193,10 @@ pub struct ProductAccount {
192193
impl ProductAccount {
193194
pub fn iter(&self) -> AttributeIter {
194195
AttributeIter {
195-
attrs: &self.attr[..(self.size as usize - PROD_HDR_SIZE)],
196+
attrs: &self.attr[..min(
197+
(self.size as usize).saturating_sub(PROD_HDR_SIZE),
198+
PROD_ATTR_SIZE,
199+
)],
196200
}
197201
}
198202
}
@@ -574,21 +578,21 @@ impl<'a> Iterator for AttributeIter<'a> {
574578
if self.attrs.is_empty() {
575579
return None;
576580
}
577-
let (key, data) = get_attr_str(self.attrs);
578-
let (val, data) = get_attr_str(data);
581+
let (key, data) = get_attr_str(self.attrs)?;
582+
let (val, data) = get_attr_str(data)?;
579583
self.attrs = data;
580584
Some((key, val))
581585
}
582586
}
583587

584-
fn get_attr_str(buf: &[u8]) -> (&str, &[u8]) {
588+
fn get_attr_str(buf: &[u8]) -> Option<(&str, &[u8])> {
585589
if buf.is_empty() {
586-
return ("", &[]);
590+
return Some(("", &[]));
587591
}
588592
let len = buf[0] as usize;
589-
let str = std::str::from_utf8(&buf[1..len + 1]).expect("attr should be ascii or utf-8");
590-
let remaining_buf = &buf[len + 1..];
591-
(str, remaining_buf)
593+
let str = std::str::from_utf8(buf.get(1..len + 1)?).ok()?;
594+
let remaining_buf = &buf.get(len + 1..)?;
595+
Some((str, remaining_buf))
592596
}
593597

594598
#[cfg(test)]
@@ -601,6 +605,11 @@ mod test {
601605
use solana_program::clock::Clock;
602606
use solana_program::pubkey::Pubkey;
603607

608+
use crate::state::{
609+
PROD_ACCT_SIZE,
610+
PROD_HDR_SIZE,
611+
};
612+
604613
use super::{
605614
PriceInfo,
606615
PriceStatus,
@@ -965,4 +974,70 @@ mod test {
965974
assert_eq!(old_b, new_b);
966975
}
967976
}
977+
978+
#[test]
979+
fn test_product_account_iter_works() {
980+
let mut product = super::ProductAccount {
981+
magic: 1,
982+
ver: 2,
983+
atype: super::AccountType::Product as u32,
984+
size: PROD_HDR_SIZE as u32 + 10,
985+
px_acc: Pubkey::new_from_array([3; 32]),
986+
attr: [0; super::PROD_ATTR_SIZE],
987+
};
988+
989+
// Set some attributes
990+
product.attr[0] = 3; // key length
991+
product.attr[1..4].copy_from_slice(b"key");
992+
product.attr[4] = 5; // value length
993+
product.attr[5..10].copy_from_slice(b"value");
994+
995+
let mut iter = product.iter();
996+
assert_eq!(iter.next(), Some(("key", "value")));
997+
assert_eq!(iter.next(), None);
998+
999+
// Check that the iterator does not panic on size misconfiguration
1000+
product.size = PROD_HDR_SIZE as u32 - 10; // Invalid size
1001+
let mut iter = product.iter();
1002+
assert_eq!(iter.next(), None); // Should not panic, just return None
1003+
1004+
product.size = PROD_ACCT_SIZE as u32 + 10; // Reset size to a size larger than the account size
1005+
let mut iter = product.iter();
1006+
assert_eq!(iter.next(), Some(("key", "value")));
1007+
while iter.next().is_some() {} // Consume the iterator
1008+
1009+
// Check that invalid len stops the iterator. This behaviour is not perfect as it
1010+
// stops reading attributes after the first invalid one but is just a safety measure.
1011+
// In this case, we set the length byte to 255 which goes beyond the size of the
1012+
// product account.
1013+
product.attr[10] = 255;
1014+
for i in 11..266 {
1015+
product.attr[i] = b'a';
1016+
}
1017+
product.attr[266] = 255;
1018+
for i in 267..super::PROD_ATTR_SIZE {
1019+
product.attr[i] = b'b';
1020+
}
1021+
let mut iter = product.iter();
1022+
assert_eq!(iter.next(), Some(("key", "value")));
1023+
assert_eq!(iter.next(), None); // No more attributes because it stopped reading the invalid value
1024+
1025+
// Make sure if the value size was set to a smaller value, it would work fine
1026+
product.attr[266] = 10;
1027+
let mut iter = product.iter();
1028+
assert_eq!(iter.next(), Some(("key", "value")));
1029+
let (key, val) = iter.next().unwrap();
1030+
assert_eq!(key.len(), 255);
1031+
for byte in key.as_bytes() {
1032+
assert_eq!(byte, &b'a');
1033+
}
1034+
assert_eq!(val, "bbbbbbbbbb"); // No more attributes because it stopped reading the invalid value
1035+
1036+
// Check that iterator stops on non-UTF8 attributes. This behaviour is not
1037+
// perfect as it stops reading attributes after the first non-UTF8 one but
1038+
// is just a safety measure.
1039+
product.attr[1..4].copy_from_slice(b"\xff\xfe\xfa");
1040+
let mut iter = product.iter();
1041+
assert_eq!(iter.next(), None); // Should not panic, just return None
1042+
}
9681043
}

0 commit comments

Comments
 (0)