Skip to content

Commit

Permalink
Limit notes lengths to mitigate potential DoS
Browse files Browse the repository at this point in the history
  • Loading branch information
akadusei committed Jul 27, 2024
1 parent cd7012c commit a05930e
Show file tree
Hide file tree
Showing 9 changed files with 77 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
### Changed
- Upgrade GitHub actions
- Limit description lengths to mitigate potential DoS
- Limit notes lengths to mitigate potential DoS

## [0.17.0] - 2024-06-02

Expand Down
1 change: 1 addition & 0 deletions docs/10-I18N.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ en:
invoice_items_empty: Invoice has no line items
invoice_not_finalized: Invoice is not finalized
invoice_not_found: Invoice does not exist
notes_too_long: Notes cannot be longer than %{max} characters
price_lte_zero: Price must be greater than zero
price_required: Price is required
quantity_lte_zero: Quantity must be greater than zero
Expand Down
18 changes: 17 additions & 1 deletion spec/bill/operations/mixins/validate_credit_note_spec.cr
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
require "../../../spec_helper"

private class SaveCreditNote < CreditNote::SaveOperation
permit_columns :invoice_id, :description, :reference, :status
permit_columns :invoice_id, :description, :notes, :reference, :status

include Bill::ValidateCreditNote
end
Expand Down Expand Up @@ -115,4 +115,20 @@ describe Bill::ValidateCreditNote do
.should(have_error "operation.error.description_too_long")
end
end

it "rejects long notes" do
user = UserFactory.create
invoice = InvoiceFactory.create &.user_id(user.id).status(:open)

SaveCreditNote.create(params(
invoice_id: invoice.id,
notes: "n" * 5000,
reference: "123",
status: :open
)) do |operation, credit_note|
credit_note.should be_nil

operation.notes.should(have_error "operation.error.notes_too_long")
end
end
end
19 changes: 19 additions & 0 deletions spec/bill/operations/mixins/validate_invoice_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ private class SaveInvoice < Invoice::SaveOperation
:business_details,
:description,
:due_at,
:notes,
:reference,
:status,
:user_details
Expand Down Expand Up @@ -150,4 +151,22 @@ describe Bill::ValidateInvoice do
.should(have_error "operation.error.description_too_long")
end
end

it "rejects long notes" do
user = UserFactory.create

SaveInvoice.create(params(
user_id: user.id,
business_details: "ACME Inc",
due_at: 1.day.from_now,
notes: "n" * 5000,
reference: "123",
status: :open,
user_details: "Mary Smith"
)) do |operation, invoice|
invoice.should be_nil

operation.notes.should(have_error "operation.error.notes_too_long")
end
end
end
19 changes: 19 additions & 0 deletions spec/bill/operations/mixins/validate_receipt_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ private class SaveReceipt < Receipt::SaveOperation
:amount,
:business_details,
:description,
:notes,
:reference,
:status,
:user_details
Expand Down Expand Up @@ -184,4 +185,22 @@ describe Bill::ValidateReceipt do
.should(have_error "operation.error.description_too_long")
end
end

it "rejects long notes" do
user = UserFactory.create

SaveReceipt.create(params(
user_id: user.id,
business_details: "ACME Inc",
amount: 90,
notes: "n" * 5000,
reference: "123",
status: :open,
user_details: "Mary Smith"
)) do |operation, receipt|
receipt.should be_nil

operation.notes.should(have_error "operation.error.notes_too_long")
end
end
end
1 change: 1 addition & 0 deletions src/bill/operations/mixins/validate_credit_note.cr
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ module Bill::ValidateCreditNote
include Bill::ValidateStatusTransition
include Bill::ValidateReference
include Bill::ValidateDescription
include Bill::ValidateNotes

private def validate_status_required
validate_required status,
Expand Down
1 change: 1 addition & 0 deletions src/bill/operations/mixins/validate_invoice.cr
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ module Bill::ValidateInvoice
include Bill::ValidateStatusTransition
include Bill::ValidateReference
include Bill::ValidateDescription
include Bill::ValidateNotes
include Lucille::ValidateUserExists

private def validate_business_details_required
Expand Down
17 changes: 17 additions & 0 deletions src/bill/operations/mixins/validate_notes.cr
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
module Bill::ValidateNotes
macro included
skip_default_validations

before_save do
validate_notes_length
end

private def validate_notes_length
max = 4094

validate_size_of notes,
max: max,
message: Rex.t(:"operation.error.notes_too_long", max: max)
end
end
end
1 change: 1 addition & 0 deletions src/bill/operations/mixins/validate_receipt.cr
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ module Bill::ValidateReceipt
include Bill::ValidateStatusTransition
include Bill::ValidateReference
include Bill::ValidateDescription
include Bill::ValidateNotes
include Lucille::ValidateUserExists

private def validate_amount_required
Expand Down

0 comments on commit a05930e

Please sign in to comment.