Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ballot id is used in the filepath but is not being validated #74

Open
JohnLCaron opened this issue Apr 27, 2024 · 1 comment
Open

Ballot id is used in the filepath but is not being validated #74

JohnLCaron opened this issue Apr 27, 2024 · 1 comment

Comments

@JohnLCaron
Copy link
Owner

From Brian Donavan:

I noticed that the part that determines the paths of the plaintext/encrypted/decrypted ballots just interpolates the id into the path without any sort of validation. since the id itself also has no validation beyond requiring that it not be empty, it could cause some unexpected behavior. for example, you could have a plaintext ballot file whose ballot_id doesn’t match the filename, resulting in an eballot- file that has no corresponding pballot- file. you could also have a ballot_id of something like /../../election_initialized which could cause the RunBatchEncryption CLI to overwrite some other config. I’m not sure whether anything silently nefarious could be achieved, but I thought it was worth bringing this to your attention since I don’t believe the system should be letting ballot_id in the input ballots determine things like the paths being written to ElectionRecordJsonPaths.kt:

    fun plaintextBallotPath(ballotDir: String, ballotId: String): String {
        val id = ballotId.replace(" ", "_")
        return "$ballotDir/$PLAINTEXT_BALLOT_PREFIX$id$JSON_SUFFIX"
    }
@JohnLCaron
Copy link
Owner Author

John:

Hi Brian, thats a good point. It is convenient to allow API access to a ballot by ballot id, rather than having to know the correct filename. Possible changes that occur to me:

  • Validate the ballot_id in BallotInputValidation, which would allow the ballot to be rejected and put into a directory to be examined by humans. The idea is to detect and quarentine the attack.
  • Do a better job of cleaning the ballot_id so nothing bad can happen. This could be done in plaintextBallotPath, which would just ignore the attack. although it could log an error or warning message.
  • Use the hash of the ballot id in the filename, or something similar.

brian:

I guess I wonder whether it makes more sense to be much stricter about the filenames (and probably using hashing a la #3) or to simply let the filenames be whatever and just use the same name but have the encrypted ballots be in another directory. I don’t have a strong opinion because my use case is really just “encrypt a single ballot from one file and get the result from the single generated file”, which is likely better served by an API not based on the file system

right now the BMD (voter terminal) is handling generating a ballot ID, so it’s not a problem with the system as a whole as currently configured

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant