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

Improve: cxx/ClassSys.cxx:classSysGetOwnPath(): for _Termux_, use procfs to return our true path. #26

Closed
SwuduSusuwu opened this issue Nov 23, 2024 · 3 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@SwuduSusuwu
Copy link
Owner

SwuduSusuwu commented Nov 23, 2024

virusAnalysisTests() uses ClassPortableExecutable() to load this, with the result that tests on Termux now always output the same offset+hash (since /proc/self/exe is /apex/com.android.runtime/bin/linker64).

Rationale: This would explain how (up until commit ?virusAnalysisTests(): if Linux, use procfs:@5f0ffd8), with each build, virusAnalysisTests()'s results showed a new offset+hash (

[./cxx/VirusAnalysis.cxx:239: Notice: signatureAnalysis(/*.file =*/ "/data/data/com.termux/files/home/SubStack/bin/a.out", /*.fileHash =*/ 0x1f2bcd741eb53ad61d715876794f04bf1bcdff63d2a03b2d78ec1283abd323d0) {return virusAnalysisAbort;} /* due to signature 0x74657374 found at offset=173170. You should treat this as a virus detection if this was not a test. */]
[./cxx/VirusAnalysis.cxx:240: Notice: signatureAnalysis(/*.file =*/ "/data/data/com.termux/files/home/SubStack/bin/a.out", /*.fileHash =*/ 0x96ac658655c1d9af12133771cdb0a22370ee6da84c4c76c0a665ced3c8f320ae) {return virusAnalysisAbort;} /* due to signature 0x74657374 found at offset=174578. You should treat this as a virus detection if this was not a test. */]

,) as opposed to the post-procfs outputs (

[./cxx/VirusAnalysis.cxx:243: Notice: signatureAnalysis(/*.file =*/ "/proc/self/exe", /*.fileHash =*/ 0x1e5b58a58065fd4863e2a84d0f418516ed8e91ce785f1a5a73741313123d82f7) {return virusAnalysisAbort;} /* due to signature 0x74657374 found at offset=20280. You should treat this as a virus detection if this was not a test. */]
[./cxx/VirusAnalysis.cxx:240: Notice: signatureAnalysis(/*.file =*/ "/proc/self/exe", /*.fileHash =*/ 0x1e5b58a58065fd4863e2a84d0f418516ed8e91ce785f1a5a73741313123d82f7) {return virusAnalysisAbort;} /* due to signature 0x74657374 found at offset=20280. You should treat this as a virus detection if this was not a test. */

) which use the /proc/self/exe path (and always give the same offset+hash.)
Use of the which linker64 path gives those results too:

[./cxx/VirusAnalysis.cxx:240: Notice: signatureAnalysis(/*.file =*/ "/apex/com.android.runtime/bin/linker64", /*.fileHash =*/ 0x1e5b58a58065fd4863e2a84d0f418516ed8e91ce785f1a5a73741313123d82f7) {return virusAnalysisAbort;} /* due to signature 0x74657374 found at offset=20280. You should treat this as a virus detection if this was not a test. */]

, thus am sure that this is due to how Termux loads all executables through linker64.
Have not labelled this as regression since all it does is cause one of the unit tests to print unexpected results for offset+hash (which does not alter the unit test's return value).
Local tests show that if you use readlink or realpath, you get the true path.
It was GitHub's code scan (https://github.com/SwuduSusuwu/SubStack/security/code-scanning/1277) which prompted the switch from argv[0] to /proc/self/exe at 5f0ffd8

@SwuduSusuwu
Copy link
Owner Author

SwuduSusuwu commented Nov 23, 2024

It was issue #25 (commit d50262f) which moved /proc/self/exe into the new function classSysGetOwnPath() (which has other routes; if Windows, GetModulePathName(), if unknown system, argv[0]).

@SwuduSusuwu
Copy link
Owner Author

The use of loader64 to execute all programs was due to Google's rules.
Note that this just affects Termux from Google Store; the open source Termux has a normal loader such that /proc/self/exe acts as argv[0].

SwuduSusuwu added a commit that referenced this issue Nov 23, 2024
	so that `virusAnalysisTests()` outputs the offset+hash of `argv[0]`.
	Note that just the _Google Store_ version of _Termux_ output the wrong offset+hash (due to _Google_'s rules: termux/termux-exec#24).
	In the future (if more is done with our path than output example offsets+hashes), it is possible that this fix will have more value.
	If other programs use `classSysGetOwnPath()` on _Termux_, this can improve those.

?`cxx/ClassSys.cxx`: ?`classSysGetOwnPath()`: the fix was to use `readlink` to return true path.
?`cxx/ClassSys.hxx`: ?`classSysGetOwnPath()`, ?`classSysFopenOwnPath()`: comments improved.
?`cxx/VirusAnalysis.cxx`: typo fixes.

Closes issue #26 (Improve: cxx/ClassSys.cxx:classSysGetOwnPath(): for _Termux_, use `procfs` to return our true path.)
Is followup to: d50262f (+`classSysGetOwnPath()`, +`classSysFopenOwnPath()`), 5f0ffd8 (?`virusAnalysisTests()`: if Linux, use procfs: this should close https://github.com/SwuduSusuwu/SubStack/security/code-scanning/1277).

?`posts/VirusAnalysis.md`:
	Include all this.
	Remove accidental duplicate comments at end of file.
SwuduSusuwu added a commit that referenced this issue Nov 23, 2024
	so that `virusAnalysisTests()` outputs the offset+hash of `argv[0]`.
	Note that just the _Google Store_ version of _Termux_ output the wrong offset+hash (due to _Google_'s rules: termux/termux-exec#24).
	In the future (if more is done with our path than output example offsets+hashes), it is possible that this fix will have more value.
	If other programs use `classSysGetOwnPath()` on _Termux_, this can improve those.

?`cxx/ClassSys.cxx`: ?`classSysGetOwnPath()`: the fix was to use `readlink` to return true path.
?`cxx/ClassSys.hxx`: ?`classSysGetOwnPath()`, ?`classSysFopenOwnPath()`: comments improved.
?`cxx/VirusAnalysis.cxx`: typo fixes.

Closes issue #26 (Improve: cxx/ClassSys.cxx:classSysGetOwnPath(): for _Termux_, use `procfs` to return our true path.)
Is followup to: d50262f (+`classSysGetOwnPath()`, +`classSysFopenOwnPath()`), 5f0ffd8 (?`virusAnalysisTests()`: if Linux, use procfs: this should close https://github.com/SwuduSusuwu/SubStack/security/code-scanning/1277).

?`posts/VirusAnalysis.md`:
	Include all this.
	Remove accidental duplicate comments at end of file.
@SwuduSusuwu
Copy link
Owner Author

SwuduSusuwu commented Nov 23, 2024

StackOverflow says stdio.h has PATH_MAX; auto build failed since GitHub's runner does not have PATH_MAX from such #include.
Since commits which fail to build cannot use git bisect, f331489 was replaced with af5226c (which has #include "limits.h" /* PATH_MAX */) which closes this.

If you used git pull in the short temporal window with f331489, just execute git pull --rebase (so that you tip is now af5226c)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

1 participant