Skip to content

Commit

Permalink
Initial commit
Browse files Browse the repository at this point in the history
  • Loading branch information
rcstanciu authored Jul 15, 2024
0 parents commit 1ceba16
Show file tree
Hide file tree
Showing 701 changed files with 48,370 additions and 0 deletions.
409 changes: 409 additions & 0 deletions .data/sync_issues.py

Large diffs are not rendered by default.

70 changes: 70 additions & 0 deletions .data/validate_changes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import os

exception_filenames = [".data", ".git", ".github", "README.md", "Audit_Report.pdf", "comments.csv", ".gitignore"]

def main():
added_files = os.environ.get("ADDED_FILES")
modified_files = os.environ.get("MODIFIED_FILES")
renamed_files = os.environ.get("RENAMED_FILES")
removed_files = os.environ.get("REMOVED_FILES")

if added_files != "":
added_files = [
x
for x in added_files.split(" ")
if not any(
y in x
for y in exception_filenames
)
]
else:
added_files = []

if modified_files != "":
modified_files = [
x
for x in modified_files.split(" ")
if not any(
y in x
for y in exception_filenames
)
]
else:
modified_files = []

if renamed_files != "":
renamed_files = [
x
for x in renamed_files.split(" ")
if not any(
y in x
for y in exception_filenames
)
]
else:
renamed_files = []

if removed_files != "":
removed_files = [
x
for x in removed_files.split(" ")
if not any(
y in x
for y in exception_filenames
)
]
else:
removed_files = []

print("MODIFIED FILES")
print(modified_files)

if len(modified_files) > 0:
print("❌ File contents should not be altered.")
exit(1)

print("✅ File contents have not be altered.")


if __name__ == "__main__":
main()
144 changes: 144 additions & 0 deletions .data/validate_filesystem.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
import os
import re
import csv

total_issues = None
comment_filename = "comments.csv"
exception_filenames = [
".data",
".git",
".github",
"README.md",
"Audit_Report.pdf",
".gitkeep",
".gitignore",
comment_filename,
]


def consume_comment_file():
with open(comment_filename) as f:
try:
reader = csv.DictReader(f)
except Exception:
return ["Unable to consume %s" % comment_filename]

if not reader.fieldnames or reader.fieldnames != ["issue_number", "comment"]:
return ["Incorrect csv header, expected `issue_number,comment`"]

errors = []
for row in reader:
try:
issue_number = int(re.match(r"(\d+)", row["issue_number"]).group(0))
except Exception:
errors.append("Unable to extract issue number from %s" % row)
continue
if issue_number < 1 or issue_number > total_issues:
errors.append("Issue %s should not be in csv" % issue_number)

comment = row.get("comment")
if not comment or len(comment) == 0:
errors.append("Empty comment on issue %s in the csv" % issue_number)
return errors


def main():
global total_issues

try:
total_issues = int(os.environ.get("TOTAL_ISSUES"))
except:
print("TOTAL_ISSUES secret not set.")
return

# Store all the errors found
errors = []
# Store all the issues read
issues = []

def process_directory(path):
nonlocal issues
print("Directory %s" % path)

# Get the items in the directory
items = [x for x in os.listdir(path) if x not in exception_filenames]

directory_has_report = False
for item in items:
print("- Item %s" % item)
is_dir = os.path.isdir(os.path.join(path, item))

if is_dir:
matches = [
r"^(H|M|High|Medium|GH|General-Health|GeneralHealth)-\d+$",
r"^\d+-(H|M|High|Medium|GH|General-Health|GeneralHealth)$",
r"^false$",
r"^invalid$",
]
correctly_formatted = any(
re.match(pattern, item, re.IGNORECASE) for pattern in matches
)
if (
not any([x in path for x in ["invalid", "false"]])
and not correctly_formatted
):
errors.append("Directory %s is not formatted properly." % item)
else:
process_directory(os.path.join(path, item))
else:
if not re.match(r"^\d+(-best)?.md$", item):
errors.append("File %s is not formatted properly." % item)
continue

# Check if the file is the best report
if "-best" in item:
if not directory_has_report:
directory_has_report = True
else:
errors.append(
"Directory %s has multiple best reports marked." % path
)

# Extract issue number from the file name
issue_number = int(re.match(r"(\d+)", item).group(0))

# Check if the issue was already found
if issue_number in issues:
errors.append("Issue %s exists multiple times." % issue_number)
else:
issues.append(issue_number)

if (
path != "."
and not any(x in path for x in ["invalid", "false"])
and not directory_has_report
and len(items) > 1
):
errors.append("Directory %s does not have a best report selected." % path)

# Start processing from the root
process_directory(".")

expected_issues = [x + 1 for x in range(total_issues)]
# Check if all issues are found in the repo
for x in expected_issues:
if x not in issues:
errors.append("Issue %s not found in the repo." % x)
# Check if there are no additional issues added
for x in issues:
if x not in expected_issues:
errors.append("Issue %s should not be in the repo." % x)

if os.path.exists(comment_filename):
errors.extend(consume_comment_file())

if len(errors) > 0:
for error in errors:
print("❌ %s" % error)
exit(1)

print("✅ Repo structure is valid.")


if __name__ == "__main__":
main()
19 changes: 19 additions & 0 deletions .github/workflows/sync-issues.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
name: sync-files-to-issues
run-name: sync-files-to-issues
on:
workflow_dispatch:
permissions: write-all
jobs:
sync-issues:
runs-on: ubuntu-latest
steps:
- name: Checkout the repository
uses: actions/checkout@v3
- name: Setup Python
uses: actions/setup-python@v4
with:
python-version: "3.8"
- name: Setup Python dependencies
run: pip install PyGithub==1.55
- name: Sync the issues
run: GITHUB_TOKEN=${{ secrets.GITHUB_TOKEN }} GITHUB_REPOSITORY=$GITHUB_REPOSITORY GITHUB_RUN_NUMBER=$GITHUB_RUN_NUMBER python .data/sync_issues.py
43 changes: 43 additions & 0 deletions .github/workflows/validate-judging-repo.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
name: validate-judging-repo
run-name: validate-judging-repo
on:
push:
branches:
- "main"
workflow_dispatch:
jobs:
validate-changes:
if: "!contains(github.event.commits[0].message, 'Initial commit')"
runs-on: ubuntu-latest
steps:
- name: Checkout the repository
uses: actions/checkout@v3
- uses: jitterbit/get-changed-files@v1
id: changes
with:
format: space-delimited
token: ${{ secrets.GITHUB_TOKEN }}
- name: Setup Python
uses: actions/setup-python@v4
with:
python-version: "3.8"
- name: Validate changes
run: >
ADDED_FILES="${{ steps.changes.outputs.added }}"
MODIFIED_FILES="${{ steps.changes.outputs.modified }}"
RENAMED_FILES="${{ steps.changes.outputs.renamed }}"
REMOVED_FILES="${{ steps.changes.outputs.removed }}"
python .data/validate_changes.py
validate-filesystem:
runs-on: ubuntu-latest
steps:
- name: Checkout the repository
uses: actions/checkout@v3
- name: Setup Python
uses: actions/setup-python@v4
with:
python-version: "3.8"
- name: Validate filesystem structure
run: >
TOTAL_ISSUES="${{ secrets.TOTAL_ISSUES }}"
python .data/validate_filesystem.py
35 changes: 35 additions & 0 deletions 001/002.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
Atomic Marmalade Parakeet

Medium

# Operators cannot add amount to a position

## Summary

Operators are not allowed to call the `addToPosition()` function as its comment states, leading to unimplemented functionality.

## Vulnerability Detail

The comments of `addToPosition()` states that this function ["Can only be called by lsNFT's owner or operators"](https://github.com/sherlock-audit/2024-06-magicsea/blob/7fd1a65b76d50f1bf2555c699ef06cde2b646674/magicsea-staking/src/MlumStaking.sol#L395).

The `addToPosition()` calls the internal function `_requireOnlyOperatorOrOwnerOf()` to perform above check. However, it restricts the caller must be the owner of provided position NFT.

As it is intended to allow operators to add amount to a position, the implemenation of `_requireOnlyOperatorOrOwnerOf()` is not consistent with the design.

## Impact

Operators cannot add amount to a position.

## Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/7fd1a65b76d50f1bf2555c699ef06cde2b646674/magicsea-staking/src/MlumStaking.sol#L140-L143

https://github.com/sherlock-audit/2024-06-magicsea/blob/7fd1a65b76d50f1bf2555c699ef06cde2b646674/magicsea-staking/src/MlumStaking.sol#L398

## Tool used

Manual Review

## Recommendation

Update the function `_requireOnlyOperatorOrOwnerOf()` to grant permission to operators.
71 changes: 71 additions & 0 deletions 001/004.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
Lone Opaque Mustang

Medium

# `maxLockMultiplier` can be set by operator

## Summary

The setLockMultiplierSettings() function in the andromeda-vesting contract documentation states it should only be callable by the owner. However, the current implementation incorrectly allows the operator to call this function as well, which is unintended and results in incorrect access restrictions.

## Vulnerability Detail

The documentation of the `setLockMultiplierSettings()` function states that it must only be called by the owner. But in reality the checks also allow the operator to call the function, which is unintended.

```solidity
/**
* @dev Set lock multiplier settings
*
* maxLockMultiplier must be <= MAX_LOCK_MULTIPLIER_LIMIT
* maxLockMultiplier must be <= _maxGlobalMultiplier - _maxBoostMultiplier
*
* Must only be called by the owner
*/
function setLockMultiplierSettings(uint256 maxLockDuration, uint256 maxLockMultiplier) external {
require(msg.sender == owner() || msg.sender == _operator, "FORBIDDEN");
// onlyOperatorOrOwner: caller has no operator rights
require(maxLockMultiplier <= MAX_LOCK_MULTIPLIER_LIMIT, "too high");
// setLockSettings: maxGlobalMultiplier is too high
_maxLockDuration = maxLockDuration;
_maxLockMultiplier = maxLockMultiplier;
emit SetLockMultiplierSettings(maxLockDuration, maxLockMultiplier);
}
```

## Impact

The issue results in the access to the `setLockMultiplierSettings()` function being restricted incorrectly.

## Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/7fd1a65b76d50f1bf2555c699ef06cde2b646674/magicsea-staking/src/MlumStaking.sol#L261-L278

## Tool used

Manual Review

## Recommendation

We recommend adapting the require statement to only allow the owner to call the function.

```solidity
/**
* @dev Set lock multiplier settings
*
* maxLockMultiplier must be <= MAX_LOCK_MULTIPLIER_LIMIT
* maxLockMultiplier must be <= _maxGlobalMultiplier - _maxBoostMultiplier
*
* Must only be called by the owner
*/
function setLockMultiplierSettings(uint256 maxLockDuration, uint256 maxLockMultiplier) external {
require(msg.sender == owner(), "FORBIDDEN");
// onlyOperatorOrOwner: caller has no operator rights
require(maxLockMultiplier <= MAX_LOCK_MULTIPLIER_LIMIT, "too high");
// setLockSettings: maxGlobalMultiplier is too high
_maxLockDuration = maxLockDuration;
_maxLockMultiplier = maxLockMultiplier;
emit SetLockMultiplierSettings(maxLockDuration, maxLockMultiplier);
}
```
Loading

0 comments on commit 1ceba16

Please sign in to comment.