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

Running setup.sh on installed Pwndbg should be more clear #1394

Closed
disconnect3d opened this issue Nov 27, 2022 · 13 comments
Closed

Running setup.sh on installed Pwndbg should be more clear #1394

disconnect3d opened this issue Nov 27, 2022 · 13 comments
Assignees
Labels
enhancement For enhancements to existing features good first issue

Comments

@disconnect3d
Copy link
Member

We have this at the end of our setup.sh:

pwndbg/setup.sh

Lines 157 to 160 in f01f932

# Load Pwndbg into GDB on every launch.
if ! grep pwndbg ~/.gdbinit &> /dev/null; then
echo "source $PWD/gdbinit.py" >> ~/.gdbinit
fi

This can be confusing for users when they try to install Pwndbg for the second time but they haven't cleared the ~/.gdbinit file.

I guess we should:

  • Check for source <gdbinit.py> in ~/.gdbinit as the very first thing in setup.sh
  • If we find an entry, we should ask the user if they want to proceed and override the initializer line
  • If the user don't want to proceed, we should stop the installation
  • If the user wants to proceed, we should install all deps etc, and add the source ... line to ~/.gdbinit at the end of setup.sh
@disconnect3d disconnect3d added bug enhancement For enhancements to existing features good first issue labels Nov 27, 2022
@Cycatz
Copy link
Contributor

Cycatz commented Nov 28, 2022

@disconnect3d Could I take this?

@disconnect3d
Copy link
Member Author

@Cycatz Hi, sure.

@Cycatz
Copy link
Contributor

Cycatz commented Dec 2, 2022

@disconnect3d I have an idea: we can mark the region of the configs added by the setup script, such as ### Begin of pwndbg configs ### and ### End of pwndbg configs###.

Thus we can remove the config between the comments when user want to overwrite, and it also avoids some false positives when simply grepping "pwndbg" in .gdbinit (e.g., a comment # pwndbg falls out of the condition).

@disconnect3d
Copy link
Member Author

disconnect3d commented Dec 3, 2022

@Cycatz We can do so, but will it actually improve anything? I mean, the user could still put anything in between that and may want to have those preserved on another pwndbg install :/

@Cycatz
Copy link
Contributor

Cycatz commented Dec 3, 2022

I mean, the user could still put anything in between that and may want to have those preserved on another pwndbg install :/

You're right, the user could still put anything in between the tags.

My concern is that the condition if ! grep pwndbg ~/.gdbinit &> /dev/null; is not robust enough to handle the scenarios where a user has comments with the word "pwndbg" or has renamed the cloned directory of pwndbg. In these cases, the grep command would not be able to accurately detect the presence of pwndbg in the user's ~/.gdbinit file, which could lead to problems with the installation process.

Anyway, it's up to you to decide if the potential benefits are worth the effort of implementing this feature.

@disconnect3d
Copy link
Member Author

Shall we then look for 'pwndbg/gdbinit.py' and then fallback to gdbinit.py?

@Cycatz
Copy link
Contributor

Cycatz commented Dec 3, 2022

Shall we then look for 'pwndbg/gdbinit.py' and then fallback to gdbinit.py?

Yes, I think that's a good idea!

@Cycatz
Copy link
Contributor

Cycatz commented Dec 3, 2022

Here is my rough implementation:

# Check for the presence of the initializer line in the user's ~/.gdbinit file
if grep -q "pwndbg/gdbinit.py" ~/.gdbinit; then
    # Ask the user if they want to proceed and override the initializer line
    read -p "An initializer line was found in your ~/.gdbinit file. Do you want to proceed and override it? (y/n) " answer

    # If the user does not want to proceed, exit the script
    if [ "$answer" != "y" ]; then
        exit 0
    fi
fi

# Install all necessary dependencies

# Check for the presence of the initializer line in the user's ~/.gdbinit file again
if grep -q "pwndbg/gdbinit.py" ~/.gdbinit; then
    # If so, comment it out
    sed -i '/pwndbg\/gdbinit.py/ s/^/# /' ~/.gdbinit
fi
# Now we can append the line sourcing pwndbg's gdbinit script
echo "source $PWD/gdbinit.py" >> ~/.gdbinit

@gsingh93 gsingh93 removed the bug label Dec 30, 2022
@disconnect3d
Copy link
Member Author

@Cycatz Can you just send this as a pull request? :)

@Cycatz
Copy link
Contributor

Cycatz commented Mar 10, 2023

Sure! I will clean the file up and issue a PR in these two days.

@Cycatz
Copy link
Contributor

Cycatz commented Mar 10, 2023

Fixed in #1620

@gsingh93
Copy link
Member

@disconnect3d anything left to be done here or can this be closed?

@disconnect3d
Copy link
Member Author

I think we are good to close it. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement For enhancements to existing features good first issue
Development

No branches or pull requests

3 participants