The blog.
You can not select more than 25 topics Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
 
 
 
 

110 lines
4.9 KiB

  1. ---
  2. title: CODEpendence
  3. description: >
  4. How to surreptitiouslyinject code via submodules that use GitHub repos
  5. posted: !!timestamp '2021-07-07'
  6. created: !!timestamp '2021-07-07'
  7. time: 11:16 AM
  8. tags:
  9. - security
  10. - GitHub
  11. - git
  12. ---
  13. TL;dr: If you use submodules that point to a GitHub repo, make sure
  14. that the commit id matches an offical branch or tag, especially if
  15. upgraded via a PR or submitted patch.
  16. This issue was disclosed to GitHub via the HackerOne Bug Bounty
  17. program and resolved by them in a timely manner. The
  18. [writeup](https://www.funkthat.com/~jmg/github.submodules.hash.txt) is
  19. available and is the same one that was provided to GitHub. It contains
  20. the complete steps in more detail than this blog post does.
  21. Discovery
  22. ---------
  23. Earlier this year, I was dealing with a git repo that used submodules.
  24. I've never been a fan of them due to the extra work involved in using
  25. them. But then a thought hit me, last year, when GitHub took down
  26. youtube-dl, someone was a bit sneaky and inserted a [copy of it into
  27. GitHub's DMCA repo](https://www.reddit.com/r/programming/comments/jhlhok/someone_replaced_the_github_dmca_repo_with/).
  28. They were able to do this because there is a feature/bug in GitHub's
  29. backend, that all the commits to a forked repo are accessible in the
  30. parent repo, it's just that the branches and tags are maintained
  31. separately.<label for="sn-reason" class="margin-toggle sidenote-number">
  32. </label><input type="checkbox" id="sn-reason" class="margin-toggle"/>
  33. <span class="sidenote">This makes sense to reduce storing duplicate data
  34. if a repo is large or has many forks.</span>
  35. Verification
  36. ------------
  37. The next question, would combining these into an attack even work?
  38. What would things look like? I created a few accounts to test them,
  39. creating a project to represent a code dependancy,
  40. [depproj](https://github.com/upstream123/depproj), that would be
  41. imported into another project by another user,
  42. [proj](https://github.com/comproj/proj). Then once those were created,
  43. have a malicious user create a fork of both the
  44. [deprpoj](https://github.com/maliciousrepo/depproj) and the
  45. [proj](https://github.com/maliciousrepo/proj).
  46. Once the malicious forks were created, clone them locally. With the
  47. clones, malicious [code can be
  48. inserted](https://github.com/maliciousrepo/depproj/commit/91781e4b9e1b1c944e19db740db12304755666b5)
  49. into the depproj repo. If you look at the repo, the previous commit
  50. was done as the maliciousrepo user, but while I was working on this,
  51. I remembered that w/ git, you can set the commit author to be anything
  52. (signing helps prevent that), so this commit appears to be done by the
  53. correct upstream123 user.
  54. Once the malicious code has been inserted, the malicious user can now
  55. update the submodule of the project to the commit id of the malicious
  56. code. This is done simply by doing:
  57. ```
  58. cd depproj
  59. git fetch origin <commitid>
  60. git checkout <commitid>
  61. ```
  62. Even though the depproj still points to the upstream123 repo, because
  63. fork commits appear IN the depproj repo, the above works w/o any other
  64. changes. This is also what makes it dangerous, because the repo is not
  65. changed, it can be disguised as a simple version update.
  66. A [PR](https://github.com/comproj/proj/pull/3) is then submitted to the
  67. project being attacked. I did not control the author of commits as
  68. well as I should have, but it still is effective. If you click into
  69. the proposed change, and then click on code.c, the file changed, it'll
  70. bring you to the [change compare
  71. view](https://github.com/upstream123/depproj/compare/91781e4b9e1b1c944e19db740db12304755666b5...370d35ec5df81a16bb361111faeb665ea90de026#diff-e43700a08429a0231daba9a49ff36a118566849856da2811ae074417ebb552d0).
  72. For this demo, it was a small change, but if the project is large, it's
  73. would be easy to bury a minor flaw in lots of changes. The other thing
  74. to note about this page is that the author displayed is NOT the author
  75. of the change, but it appears that it is a legitimate change by the
  76. author of the repo. [![Screenshot of github comparing changes with a popup of the author showing the author owns this repository and committed to this repository.]([[!!images/codependence-comp-author.png]])]({{ media_url('images/codependence-comp-author.png') }})
  77. Conclusion
  78. ----------
  79. This is an interesting attack in that it leverages two features in a
  80. way that has surprising results. It demonstrates that software
  81. dependancies need to be reviewed, and vetted, and that if you're using
  82. GitHub, that just because a PR says it's updating a submodule to the
  83. new version, it doesn't mean that it is safe to simply merge in the
  84. change.
  85. Timeline
  86. --------
  87. 2021-03-31 -- Reported to GitHub via HackerOne.<br>
  88. 2021-03-31 -- More info requested and provided.<br>
  89. 2021-04-01 -- Ack'd issue and started work on fix.<br>
  90. 2021-05-04 -- GitHub determined it was low risk, but did add warning when viewing commit.<br>
  91. 2021-05-05 -- Asked GitHub for disclosure timeline.<br>
  92. 2021-06-04 -- Pinged GitHub again.<br>
  93. 2021-07-07 -- Published blog post.<br>