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

fix --since_commit parameter #108

Closed
fahrishb opened this issue May 15, 2018 · 18 comments
Closed

fix --since_commit parameter #108

fahrishb opened this issue May 15, 2018 · 18 comments

Comments

@fahrishb
Copy link

Hi, how can I contribute to this project?
I was running truffleHog and using the --since_commit parameter, however it was buggy and did not work as expected. I made a very small change, and it worked as expected. Do you accept PRs or should I just tell you the change so you can verify it?

@dxa4481
Copy link
Collaborator

dxa4481 commented May 15, 2018

What was the issue?

@fahrishb
Copy link
Author

if you pass a hash to the --since_commit parameter, it will go through the code and will not stop scanning pass that hash. its mainly because of this
truffleHog.py on line 260.

if since_commit_reached is True, it will just mark it as True and still go on with the function.
if the continue line is changed to break, it will break out of the function and stop the scan.

@dxa4481
Copy link
Collaborator

dxa4481 commented May 15, 2018

My understanding is continue in python means stop that iteration of the loop. I think the break may speed things up, but functionally if I'm not mistaken it should behave the same.

What was buggy?

@fahrishb
Copy link
Author

You can check the example here,

But before we continue, I want to clarify if I'm understanding the --since_commit parameter as what you intended it to.

For example, we have a few commit hashes

1
2
3
4
5

And I run truffleHog with parameter --since_commit 3. In my understanding, it should only scan commit 1 and commit 2, and stop when it reached commit 3. Is this the flow you intended?

If I run it with the current code, with the continue statement, it will go through all commits and skip the 3rd commit

@dxa4481
Copy link
Collaborator

dxa4481 commented May 15, 2018

after since_commit_reached is set to True, since_commit and since_commit_reached should evaluate to True each iteration, and the the 4 and 5 should not be scanned

@fahrishb
Copy link
Author

Yes, so changing continue to break should behave as we mentioned.
Can you confirm this?
I just tried running it again with continue, but it still scans the commits after the since_commit parameter. Changing it to break stops the scan at commit since_commit.

@dxa4481
Copy link
Collaborator

dxa4481 commented May 15, 2018

You haven't answered my question, which is, what exactly isn't working?

right now after since_commit_reached is set to True, since_commit and since_commit_reached should always evaluate True, and each future loop iteration should be skipped. If this is not happening, either provide me with a test case where it's not, or explain what's going wrong?

@fahrishb
Copy link
Author

yes they evaluate to True, but since you have continue, it will still go through the entire commit hash.
I already mentioned earlier the problem is that it does not stop at the since_commit parameter commit hash.
I changed continue to break on line 260 on truffleHog.py and it fixed the problem. thats why I wanted you to verify it too so we are on the same page.

@dxa4481
Copy link
Collaborator

dxa4481 commented May 15, 2018

I don't think continue does what you think it does.

Try running the following code:

>>> for i in range(10):
...     if i > 5:
...             continue
...     print(i)
...
0
1
2
3
4
5

@fahrishb
Copy link
Author

True, that is the output I get after running your code.
I think to make it a more equivalent comparison to the source code you should try i == 5 instead of i > 5 because you're comparing it with the equal sign as per line 256

if commitHash == since_commit:
    since_commit_reached = True

try this, this compares the output between using continue, and break
continue

In [7]: for letter in 'Python':     # First Example
   ...:    if letter == 'h':
   ...:       continue
   ...:    print 'Current Letter :', letter
   ...:   
Current Letter : P
Current Letter : y
Current Letter : t
Current Letter : o
Current Letter : n

break

In [6]: for letter in 'Python':     # First Example
   ...:    if letter == 'h':
   ...:       break
   ...:    print 'Current Letter :', letter
   ...:   
Current Letter : P
Current Letter : y
Current Letter : t

based on the example above, I think break is more suitable than continue for this case since we want the scan to stop when commitHash == since_commit, or from the above example, its letter == h

@dxa4481
Copy link
Collaborator

dxa4481 commented May 15, 2018

But that's not what's happening. Here's the exact code snippet

if commitHash == since_commit:
     since_commit_reached = True
if since_commit and since_commit_reached:
     prev_commit = curr_commit
     continue

since_commit_reached is set to True once, but it stays true. And that forces the second if statement to continue to evaluate True.

@milo-minderbinder
Copy link

milo-minderbinder commented May 15, 2018

My understanding is continue in python means stop that iteration of the loop. I think the break may speed things up, but functionally if I'm not mistaken it should behave the same.

FWIW, continue does not mean stop iteration of the loop, it actually means "continue execution of the loop at the next iteration" (e.g. skip the rest of this iteration of the loop, and skip straight to the next iteration). In this particular case, I think it is, roughly speaking, functionally equivalent, because the continue statement is at the top of the loop, and since_commit_reached evaluation will remain true for the remaining loop iterations, so the rest of the login inside the loop (which is basically all of the scan logic) is skipped. That said, the use of continue in this case causes the script to unnecessarily run through every single commit but effectively doing nothing after since_commit_reached is true, since it hits the continue.

break does mean stop iteration of the nearest enclosing loop.

@LewisLebentz
Copy link

@fahrishb @milo-minderbinder does this work now?

I seem to be having problems if I scan the second newest commit, it doesn't seem to find anything in the newest commit until I make another commit. So it appears to skip the first commit it finds.

@lufte
Copy link

lufte commented Feb 4, 2019

I've stumbled upon this same issue when playing around with --since_commit. I leave here a small demo of what I did: I start with an empty repo and add 3 files, each in its own commit. Everything seems to work except all of the --since_commit calls, which yield incorrect results.

I would expect to see both secrets on the first call, also both secrets on the second call and just the entropy secret on the third call. That it the expected behavior, right?

(env) lufte@lufte-thinkpad:~/projects/test-repo$ git init
Initialized empty Git repository in /home/lufte/projects/test-repo/.git/
(env) lufte@lufte-thinkpad:~/projects/test-repo$ cat no_secret
#!/usr/bin

echo normal file
(env) lufte@lufte-thinkpad:~/projects/test-repo$ git add no_secret && git commit -m "normal file"
[master (root-commit) c5bd51f] normal file
 1 file changed, 3 insertions(+)
 create mode 100644 no_secret
(env) lufte@lufte-thinkpad:~/projects/test-repo$ trufflehog --regex .
(env) lufte@lufte-thinkpad:~/projects/test-repo$ cat id_rsa
-----BEGIN RSA PRIVATE KEY-----
111111111111111111111111111111
-----END RSA PRIVATE KEY-----
(env) lufte@lufte-thinkpad:~/projects/test-repo$ git add id_rsa && git commit -m "secret ssh key"
[master 5e4bc66] secret ssh key
 1 file changed, 3 insertions(+)
 create mode 100644 id_rsa
(env) lufte@lufte-thinkpad:~/projects/test-repo$ trufflehog --regex .
~~~~~~~~~~~~~~~~~~~~~
Reason: RSA private key
Date: 2019-02-04 13:29:01
Hash: 5e4bc66c275d087dea6b828100058ac55b92cbf2
Filepath: id_rsa
Branch: origin/master
Commit: secret ssh key

-----BEGIN RSA PRIVATE KEY-----
~~~~~~~~~~~~~~~~~~~~~
(env) lufte@lufte-thinkpad:~/projects/test-repo$ cat secret
4reZuwh+q03FNsn21YfZEEnEIg+8GP+EJmTR0nYRDdZEyPxbor/WWCcBdTSAuRaivYZPJdBb0eLP
(env) lufte@lufte-thinkpad:~/projects/test-repo$ git add secret && git commit -m "add secret"
[master 43024a0] add secret
 1 file changed, 1 insertion(+)
 create mode 100644 secret
(env) lufte@lufte-thinkpad:~/projects/test-repo$ trufflehog --regex .
~~~~~~~~~~~~~~~~~~~~~
Reason: High Entropy
Date: 2019-02-04 13:29:25
Hash: 43024a0653cb890debe493f8efb0bc7afad4245d
Filepath: secret
Branch: origin/master
Commit: add secret

@@ -1 +0,0 @@
-4reZuwh+q03FNsn21YfZEEnEIg+8GP+EJmTR0nYRDdZEyPxbor/WWCcBdTSAuRaivYZPJdBb0eLP

~~~~~~~~~~~~~~~~~~~~~
~~~~~~~~~~~~~~~~~~~~~
Reason: RSA private key
Date: 2019-02-04 13:29:01
Hash: 5e4bc66c275d087dea6b828100058ac55b92cbf2
Filepath: id_rsa
Branch: origin/master
Commit: secret ssh key

-----BEGIN RSA PRIVATE KEY-----
~~~~~~~~~~~~~~~~~~~~~
(env) lufte@lufte-thinkpad:~/projects/test-repo$ git log
commit 43024a0653cb890debe493f8efb0bc7afad4245d (HEAD -> master)
Author: lufte <javierayres@gmail.com>
Date:   Mon Feb 4 13:29:25 2019 -0300

    add secret

commit 5e4bc66c275d087dea6b828100058ac55b92cbf2
Author: lufte <javierayres@gmail.com>
Date:   Mon Feb 4 13:29:01 2019 -0300

    secret ssh key

commit c5bd51f0e6aa43a8696143a569c784ed5fa352e6
Author: lufte <javierayres@gmail.com>
Date:   Mon Feb 4 13:28:35 2019 -0300

    normal file
(env) lufte@lufte-thinkpad:~/projects/test-repo$ trufflehog --regex --since_commit=c5bd51f0e6aa43a8696143a569c784ed5fa352e6 .
~~~~~~~~~~~~~~~~~~~~~
Reason: High Entropy
Date: 2019-02-04 13:29:25
Hash: 43024a0653cb890debe493f8efb0bc7afad4245d
Filepath: secret
Branch: origin/master
Commit: add secret

@@ -1 +0,0 @@
-4reZuwh+q03FNsn21YfZEEnEIg+8GP+EJmTR0nYRDdZEyPxbor/WWCcBdTSAuRaivYZPJdBb0eLP

~~~~~~~~~~~~~~~~~~~~~
(env) lufte@lufte-thinkpad:~/projects/test-repo$ trufflehog --regex --since_commit=5e4bc66c275d087dea6b828100058ac55b92cbf2 .
(env) lufte@lufte-thinkpad:~/projects/test-repo$ trufflehog --regex --since_commit=43024a0653cb890debe493f8efb0bc7afad4245d .

@prabhu
Copy link

prabhu commented Mar 13, 2019

Yeah since_commit appears to be quite buggy that you need commitHash - 2 to test the commitHash.

@caglarsayin
Copy link

Should we implement a since_commit similar feature with the date-time to the commit? It could be --since-date and we could limit the iteration on each branch with the datetime token.

@mathan94
Copy link

any idea on when will this be fixed? cheers

@rospyn
Copy link

rospyn commented Mar 20, 2020

the same happens here!
Everything seems to work, except all the options --since_commit.
I am trying to execute only on the last commit.

jpaakko added a commit to espoon-voltti/espooevents-service that referenced this issue Apr 7, 2020
Trufflehog's --since_commit option doesn't seem to work properly
(see trufflesecurity/trufflehog#108). Instead,
search for secrets only in the current feature branch's commits
by using the --branch and --max_depth options. Semantically, this
ought to be the same as using --since_commit.
angrymeir pushed a commit to angrymeir/truffleHog that referenced this issue Aug 19, 2020
As described in trufflesecurity#108 the --since_commit parameter didn't stop at the given commit.
dustin-decker pushed a commit that referenced this issue Apr 3, 2022
…108)

Bumps [github.com/aws/aws-sdk-go-v2/service/sts](https://github.com/aws/aws-sdk-go-v2) from 1.16.1 to 1.16.2.
- [Release notes](https://github.com/aws/aws-sdk-go-v2/releases)
- [Changelog](https://github.com/aws/aws-sdk-go-v2/blob/main/CHANGELOG.md)
- [Commits](aws/aws-sdk-go-v2@v1.16.1...service/efs/v1.16.2)

---
updated-dependencies:
- dependency-name: github.com/aws/aws-sdk-go-v2/service/sts
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

9 participants