search

OWASP / ZSC

OWASP ZSC - Shellcode/Obfuscate Code Generator https://www.secologist.com/
https://www.secologist.com/
Other
647 stars 217 forks source link

Windows shellcode generator not working after pep8 fixes #74

Closed Pratik151 closed 8 years ago

Pratik151 commented 8 years ago

with last commit the windows shellcode is not working. Here is the image to it http://i.imgur.com/GS6ZRkg.png I think it opcoder got messed up, have to look into it.

And I think again we have to discuss those changes as there were around 10k+ lines and In some places I think code was looking good before pep8 fixes like this (before and after ).

And at this stage I don't think we need pep8 fixes. Maybe later we can have pep8 fixes and we can also add about pep8 rules to be followed while contributing in documentation.

paraschetal commented 8 years ago

Agreed. In later stages of the project(when we have a proper testing module) we can switch to the pep8 code. Changing so many lines of code without testing might have created bugs like @Pratik151 mentioned without us even knowing them.

CodeMaxx commented 8 years ago

I made a mistake while fixing merge conflicts. Sorry for the trouble. I'll fix it right now.

CodeMaxx commented 8 years ago

@paraschetal Take a look at the second commit of my PR where I manually fixed merge conflicts. Thats where I made a mistake. The software gave correct code. Take a look at this https://github.com/zscproject/OWASP-ZSC/pull/72/commits/c48bde9dd1802b85d454e855a3d84363055b5e1f#diff-aeaa114d2de6b93b0ec2711b3bfe57fcR93

I changed the indentation by mistake. I'm really sorry. Very bad mistake on my part.

CodeMaxx commented 8 years ago

Also I don't understand why doing it at a later stage would be better. Wouldn't it be worse? More chances of getting things wrong? So this is the deal:-

  1. Add the feature you are working on in the PR without thinking about the formatting.
  2. Test your feature.
  3. Format your changes using yapf.
  4. Test the added feature again.

You'll now be sure that formatting using yapf didn't break any code since you have checked it for every feature during development. Hence formatting rules should be followed as soon as possible.

CodeMaxx commented 8 years ago

To prevent such mistakes as I did we should set up automated tests using Travis. The project is quickly developing and it would not be possible to manually check each old feature when a PR is made.

Pratik151 commented 8 years ago

@CodeMaxx I am not sure like how we can set automated test on this project? like what could be the test cases.

and yea I got indentation mistake. That fix will make this work I think. Make PR of fix.

CodeMaxx commented 8 years ago

Just like you said you got the wrong output for the shellcode... That is one possibility...We can match the shellcode against a working shellcode(which worked on your machine when you made the PR) For e.g. if we had such tests then my second commit would not have passed that test and I would have immediately found the error...There can be other ways. We should properly discuss how exactly do we want this to be done.

Read through this to get the basic idea for CI(https://about.gitlab.com/2015/02/03/7-reasons-why-you-should-be-using-ci/)

Ali-Razmjoo commented 8 years ago

I think we need to look into other parts of software too, maybe it makes more bugs. @Pratik151 let me know if #76 works fine.

Ali-Razmjoo commented 8 years ago

let me know if there is more bugs, thanks.