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

eval of text strings #654

Open
CKrawczyk opened this issue Nov 23, 2022 · 1 comment
Open

eval of text strings #654

CKrawczyk opened this issue Nov 23, 2022 · 1 comment
Labels

Comments

@CKrawczyk
Copy link
Collaborator

re-post from talk: https://www.zooniverse.org/talk/1322/2699917

I've noticed that panoptes_aggregation, while reducing, sometimes attempts to eval annotations. The relevant code is in csv_utils.py:unflatten_data.

It seems that any string containing a '{' or '[' character will be eval'd. I can see the sense of this for things like dropdowns, where we are actually reading a Python data structure.

It seems dangerous for text reducers, though. I think it is not having too much impact on my own project (issue here with more detail: nationalarchives/hms-nhs-scripts#34). It seems like in practice most free-text strings that trigger the attempted eval are not valid Python structures and so the original string is just kept and no harm is done. For me the most obvious problem is that "[...]" gets transformed into a list containing an Ellipsis object, resulting in "[...]" becoming "[Ellipsis]" in my output. Which is annoying, but not the worst.

Of course I don't know the panoptes_aggregation code very well, so I could be missing something, but I suspect that this is a bug and thought I should let you know. Even if it is not a bug, I would suggest that ast.literal_eval may be a better way to go, both for security and for correctness.

All of this is based on the v4.0.0 tag.

@CKrawczyk CKrawczyk added the bug label Nov 23, 2022
@bogden1
Copy link

bogden1 commented Nov 23, 2022

Just to clarify, if I'm understanding this correctly then I would suggest:

  1. Don't do this at all for text types (or any other tasks where volunteers enter arbitrary text)
  2. ast.literal_eval for all other cases, as this will only accept data structures and not execute code

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

No branches or pull requests

2 participants