CVE - Remote code execution in test suite.

[BillSchumacher]

Please read the bug submission guidelines before submitting a bug.

Not following guidelines may result in your bug being ignored and/or closed.

Description of Bug
Remote code execution is possible via rouge commit or hidden logic.

Code to Reproduce
See PR.

IMPORTANT: Remember to anonymize your code. Be sure to replace API keys/Client IDs with placeholders. Also, never, ever share the contents of your token file.

Expected Behavior
Code does not execute arbitrarily on my system.

Actual Behavior
Code executes arbitrarily when running test suite.

Error/Exception Log, If Applicable
See here to learn how to turn on debug logging: https://tda-api.readthedocs.io/en/latest/help.html

[BillSchumacher]

The implementation of this is very dangerous and highly suspect.

[alexgolec]

So let me get this straight: you're executing code arbitrarily on my system by… editing the library? This is not exactly CVE material.

Don’t get me wrong, I get the principle of what you’re trying to argue. The library is in fact generating text that happens to represent code, and you have found a way to manipulate that text. But generating text does not constitute a security vulnerability.

[BillSchumacher]

By passing a value to a function, I could have done this a number of ways as could have you.

[BillSchumacher]

If you want to see a sneakier example I can put that together too.

[BillSchumacher]

You asked me to show you where this was ever executed by the library, there it is.

[BillSchumacher]

I'm not saying you're a bad person, but I will say it would be very easy for you to sneak in nefarious logic.

[alexgolec]

While I think you have pointed out something good, which is that there is an opportunity here to replace this exec statement with a compiler attempt, I have to admit your demeanor and rudeness make me less than eager to continue working with you.

If you want to redeem yourself by making yourself useful, feel free to send me a PR that replaces this exec call with a call to the following:

https://stackoverflow.com/questions/37012947/how-to-tell-if-a-single-line-of-python-is-syntactically-valid

[BillSchumacher]

Your lax attitude about security makes me not want to work with you either, I was simply protecting someone that may not know what this code was capable of.

[BillSchumacher]

Consider the people that use your library and their users, the most probable victims, over my rudeness.

[alexgolec]

Update: running exec in the test suite is in fact correct behavior because the test needs to verify that the generated object is equal to the one that was passed in. Working as intended, no CVE granted.

[BillSchumacher]

Lol, sir... let's talk about pickle then.

[BillSchumacher]

You unfortunately don't get to decide what is a risk and what is not. exec is almost never the correct answer.

[BillSchumacher]

A warning at a minimum about the security risks of using pickle should be posted.

[BillSchumacher]

This kind of response to something that would ultimately affect me directly by running tests on your code base is unacceptable. Considering the amount of money involved.

[r4m0n]

This is not a chat room, please stop spamming people following this library.

[BillSchumacher]

Ah, well I tried here. Good luck folks.

[alexgolec]

+1 to @r4m0n. Please note that if the next interaction you have with this library or its community is not constructive, I will block you from interacting with us.