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:shellcraft mips dupsh wrong redirect #1182

Merged
merged 3 commits into from Feb 2, 2019

Conversation

Projects
None yet
3 participants
@leommxj
Copy link
Contributor

leommxj commented Aug 9, 2018

No description provided.

@Arusekk

This comment has been minimized.

Copy link
Collaborator

Arusekk commented Aug 9, 2018

This is actually caused by shellcraft.mips.linux.dup() missing, which is present on some other architectures, but still under different names (is this intended? i386.dupio(), but amd64.dup() and thumb.dup()).

/* dup() file descriptor ${sock} into stdin/stdout/stderr */

/* dup() file descriptor ${sock} into stdin/stdout/stderr */

/* dup() file descriptor ${sock} into stdin/stdout/stderr */

@zachriggle

This comment has been minimized.

Copy link
Contributor

zachriggle commented Sep 17, 2018

Sorry for the long delay. Would you mind making this a decrementing loop to save space? Similar to how i386.linux.dupio works:

<%
  dup       = common.label("dup")
  looplabel = common.label("loop")
%>

    /* dup() file descriptor ${sock} into stdin/stdout/stderr */
${dup}:
    ${mov('ebx', sock)}
    ${mov('ecx', 3)}
${looplabel}:
    dec ecx

    ${dup2('ebx', 'ecx')}
    jnz ${looplabel}

Separately, please rename this to dupio and update dupsh accordingly? The reason for the rename was to avoid conflicts with the syscall name, it looks like it wasn't applied everywhere.

@zachriggle

This comment has been minimized.

Copy link
Contributor

zachriggle commented Sep 17, 2018

Also, can you add a test for this so that we can be sure that it does not regress? See the "Example" block in mov.asm for how tests work.

@zachriggle zachriggle self-assigned this Feb 2, 2019

@zachriggle zachriggle merged commit 5fa4fcd into Gallopsled:dev Feb 2, 2019

3 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.06%) to 58.614%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment