Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions contracts/events/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ pub enum Error {

OpAlreadySeen = 60,

// Also returned by append_submission's cap check — the enum is at
// the 50-case XDR cap, so the hackathon submission cap reuses this
// rather than adding a variant.
TooManyContributors = 61,

CancellationNotStarted = 62,
Expand Down
14 changes: 14 additions & 0 deletions contracts/events/src/event_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ pub const PRIZE_CLAIM_WINDOW_SECS: u64 = 90 * 24 * 60 * 60;

pub const MAX_APPLICANTS_PER_EVENT: u32 = 5_000;
pub const MAX_CONTRIBUTORS_PER_EVENT: u32 = 5_000;
pub const MAX_SUBMISSIONS_PER_EVENT: u32 = 5_000;
pub const MAX_CONTENT_URI_LEN: u32 = 256;
Comment thread
coderabbitai[bot] marked this conversation as resolved.

pub const MAX_REFUNDS_PER_BATCH: u32 = 25;

Expand Down Expand Up @@ -546,6 +548,12 @@ pub fn submit(

applicant.require_auth();

// Reused rather than adding a new variant — stays inside the
// contracterror 50-variant cap (see BACKLOG.md L7 for precedent).
if content_uri.len() > MAX_CONTENT_URI_LEN {
return Err(Error::TitleTooLong);
}

let existing = storage::get_submission(env, event_id, &applicant);

if existing.is_none() {
Expand All @@ -555,6 +563,12 @@ pub fn submit(
}
}

// Reserve the slot before writing — Hackathon events have
// needs_application == false, so any address can call submit() with no
// prior gate. Without this cap, an attacker spamming fresh addresses
// grows persistent storage / rent burden without bound.
storage::append_submission(env, event_id, &applicant, MAX_SUBMISSIONS_PER_EVENT)?;

let submitted_at = existing
.as_ref()
.map(|s| s.submitted_at)
Expand Down
48 changes: 48 additions & 0 deletions contracts/events/src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -430,8 +430,56 @@ pub fn set_submission(env: &Env, id: u64, applicant: &Address, submission: &Subm
}

pub fn remove_submission(env: &Env, id: u64, applicant: &Address) {
// Idempotent: a no-op when there is nothing to remove, symmetrically
// with append_submission, so a caller that skips its own existence
// check can't silently corrupt the counter by decrementing for an
// applicant that never had a submission.
if get_submission(env, id, applicant).is_none() {
return;
}

let key = DataKey::EventSubmission(id, applicant.clone());
env.storage().persistent().remove(&key);

let count_key = DataKey::EventSubmissionCount(id);
let next = submission_count(env, id).saturating_sub(1);
if next == 0 {
env.storage().persistent().remove(&count_key);
} else {
env.storage().persistent().set(&count_key, &next);
touch_event_persistent(env, &count_key);
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

pub fn submission_count(env: &Env, id: u64) -> u32 {
let key = DataKey::EventSubmissionCount(id);
let n: Option<u32> = env.storage().persistent().get(&key);
if n.is_some() {
touch_event_persistent(env, &key);
}
n.unwrap_or(0)
}

/// Reserve a submission slot against the per-event cap before writing the
/// entry (mirrors `append_contributor`/`append_applicant`). A no-op when the
/// applicant already has a submission — re-submission updates the existing
/// entry in place and must not recount against the cap.
///
/// Returns `Error::TooManyContributors` on cap-exceed — reused rather than
/// a new variant since the errors enum is at the 50-case XDR cap.
pub fn append_submission(env: &Env, id: u64, addr: &Address, cap: u32) -> Result<(), Error> {
if get_submission(env, id, addr).is_some() {
return Ok(());
}
let cur = submission_count(env, id);
if cur >= cap {
return Err(Error::TooManyContributors);
}
let count_key = DataKey::EventSubmissionCount(id);
let next = cur.saturating_add(1);
env.storage().persistent().set(&count_key, &next);
touch_event_persistent(env, &count_key);
Ok(())
}

// ============================================================
Expand Down
135 changes: 134 additions & 1 deletion contracts/events/src/tests/hackathon_pillar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ use soroban_sdk::{
token, Address, BytesN, Env, Map, String,
};

use crate::types::{CreateEventParams, EventStatus, Pillar, ReleaseKind, WinnerSpec};
use crate::errors::Error;
use crate::event_ops::{MAX_CONTENT_URI_LEN, MAX_SUBMISSIONS_PER_EVENT};
use crate::storage;
use crate::types::{CreateEventParams, DataKey, EventStatus, Pillar, ReleaseKind, WinnerSpec};
use crate::{EventsContract, EventsContractClient};

use boundless_profile::{ProfileContract, ProfileContractClient};
Expand All @@ -18,6 +21,7 @@ const FEE_AMOUNT: i128 = (TOTAL_BUDGET * FEE_BPS as i128) / 10_000_i128;
struct Ctx<'a> {
env: Env,
events: EventsContractClient<'a>,
events_id: Address,
profile: ProfileContractClient<'a>,
owner: Address,
applicant: Address,
Expand Down Expand Up @@ -64,6 +68,7 @@ fn setup<'a>() -> Ctx<'a> {
Ctx {
env,
events,
events_id,
profile,
owner,
applicant,
Expand All @@ -73,6 +78,15 @@ fn setup<'a>() -> Ctx<'a> {
}
}

fn expect_op_err<T, E>(
result: Result<Result<T, E>, Result<Error, soroban_sdk::InvokeError>>,
) -> Error {
match result {
Err(Ok(e)) => e,
_ => panic!("expected contract error"),
}
}

fn single_winner_dist(env: &Env) -> Map<u32, u32> {
let mut m = Map::new(env);
m.set(1, 100);
Expand Down Expand Up @@ -282,6 +296,125 @@ fn withdraw_submission_removes_anchor() {
assert!(res.is_err(), "withdrawn submission is no longer readable");
}

#[test]
fn remove_submission_on_nonexistent_entry_does_not_corrupt_counter() {
let ctx = setup();
let id = create_hackathon(&ctx);

let submitter = Address::generate(&ctx.env);
ctx.events.submit(
&id,
&submitter,
&String::from_str(&ctx.env, "ipfs://Qm.../v1.json"),
&BytesN::random(&ctx.env),
);

// ctx.applicant never submitted — calling the low-level storage helper
// directly for it must be a no-op, not decrement the counter that
// `submitter`'s real submission incremented.
ctx.env.as_contract(&ctx.events_id, || {
storage::remove_submission(&ctx.env, id, &ctx.applicant);
});

let count = ctx
.env
.as_contract(&ctx.events_id, || storage::submission_count(&ctx.env, id));
assert_eq!(
count, 1,
"removing a nonexistent submission must not corrupt the counter"
);
}

#[test]
fn withdraw_submission_frees_the_slot_for_future_submitters() {
let ctx = setup();
let id = create_hackathon(&ctx);

let uri = String::from_str(&ctx.env, "ipfs://Qm.../v1.json");
ctx.events
.submit(&id, &ctx.applicant, &uri, &BytesN::random(&ctx.env));
ctx.events
.withdraw_submission(&id, &ctx.applicant, &BytesN::random(&ctx.env));

let count = ctx
.env
.as_contract(&ctx.events_id, || storage::submission_count(&ctx.env, id));
assert_eq!(
count, 0,
"withdrawing a submission must free its slot against the cap"
);
}

#[test]
fn submit_beyond_cap_reverts() {
let ctx = setup();
let id = create_hackathon(&ctx);

// Fast-forward the per-event counter directly instead of performing
// MAX_SUBMISSIONS_PER_EVENT real submissions from distinct addresses.
ctx.env.as_contract(&ctx.events_id, || {
ctx.env.storage().persistent().set(
&DataKey::EventSubmissionCount(id),
&MAX_SUBMISSIONS_PER_EVENT,
);
});

let uri = String::from_str(&ctx.env, "ipfs://Qm.../overflow.json");
let op = BytesN::random(&ctx.env);
let err = expect_op_err(ctx.events.try_submit(&id, &ctx.applicant, &uri, &op));
assert_eq!(
err,
Error::TooManyContributors,
"a submission at cap + 1 must revert"
);
}

#[test]
fn submit_oversized_content_uri_reverts() {
let ctx = setup();
let id = create_hackathon(&ctx);

let too_long = "x".repeat((MAX_CONTENT_URI_LEN + 1) as usize);
let uri = String::from_str(&ctx.env, &too_long);
let op = BytesN::random(&ctx.env);
let err = expect_op_err(ctx.events.try_submit(&id, &ctx.applicant, &uri, &op));
// Reused rather than a new variant — stays inside the contracterror
// 50-variant cap (see BACKLOG.md L7 for precedent).
assert_eq!(
err,
Error::TitleTooLong,
"content_uri beyond MAX_CONTENT_URI_LEN must revert"
);
}

#[test]
fn resubmit_by_existing_applicant_does_not_increment_submission_count() {
let ctx = setup();
let id = create_hackathon(&ctx);

let uri_a = String::from_str(&ctx.env, "ipfs://Qm.../v1.json");
ctx.events
.submit(&id, &ctx.applicant, &uri_a, &BytesN::random(&ctx.env));

let count_after_first = ctx
.env
.as_contract(&ctx.events_id, || storage::submission_count(&ctx.env, id));
assert_eq!(count_after_first, 1);

let uri_b = String::from_str(&ctx.env, "ipfs://Qm.../v2.json");
ctx.events
.submit(&id, &ctx.applicant, &uri_b, &BytesN::random(&ctx.env));

let count_after_second = ctx
.env
.as_contract(&ctx.events_id, || storage::submission_count(&ctx.env, id));
assert_eq!(
count_after_second, 1,
"re-submission by an existing applicant updates in place and must not \
recount against the cap"
);
}

// ============================================================
// select_winners — distribution (happy paths)
// ============================================================
Expand Down
3 changes: 3 additions & 0 deletions contracts/events/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,9 @@ pub enum DataKey {

// Appended for two-step manager rotation to preserve key discriminants.
PendingManager(u64),

// Appended to cap per-event submission storage growth (security fix).
EventSubmissionCount(u64),
}

// ============================================================
Expand Down
Loading