Hi, On Fri, Aug 02, 2024 at 01:04:32PM +0200, a1ex@dismail.de wrote:
I think a simple approach to test this is to find out if we cannot read the ~/.ssh/id_rsa private keys from Dillo or plugins by any means. So far I think we could read them by forging a call to the file plugin and maybe by forking a new process that doesn't have the unveil() protections and then reading ~/.ssh/id_rsa from there.
Right, but this attack vector seems pretty unlikely, and assumes the system is already somehow compromised (by calling a malicious program on the filesystem in an unveiled location).
I think an RCE bug could allow an attacker to write an exploit in an HTML document (or image or any other thing parsed from the web) that would cause Dillo to execute the fork() syscall and then access those files and transfer the content to a remote host. See: https://en.wikipedia.org/wiki/Return-oriented_programming If this is the case, unless we prevent Dillo from being able to fork I don't think the unveil(2) protection is enough without pledge(2) to prevent this attack. We can test it by putting this code after the last unveil() call, as if Dillo was running an exploit: int ret = fork(); if (ret < 0) { perror("fork failed"); } else if (ret == 0) { /* Child */ FILE *f = fopen("/home/<theuser>/.ssh/id_rsa", "r"); // Adjust path if (f == NULL) { perror("game over exploit, fopen failed"); } else { int c; while ((c = fgetc(f)) != EOF) fputc(c, stdout); fclose(f); } exit(0); } If you see your SSH key in the stdout, the unveil() protection alone is not enough to mitigate this attack (check that the key exists first, I used id_rsa). This would also confirm that unveil() settings don't get inherited in forked children.
To-Do: - Add prefs parsing to plugins to get 'save_dir' (may need help here)
I assume you could reuse the same prefs parser from Dillo, but we would need to link the DPIs with some of the code that is now only being linked in the browser.
I have made some efforts to do that, but was not successful yet. I included an example earlier of downloads.cc where I made some progress, but still something missing.
I still wonder if it makes more sense to just use a fixed location (~/Downloads) if we are calling unveil, and override the save_dir with that. This ties back to my response above.
Maybe for now we can add a simple implementation of the parser just to parse the "save_dir" and the new "enable_unveil" (or similar) option, like dpid is doing for dpi_dir. Parsing the complete dillorc preferences requires splitting the preferences code into something that can be linked standalone in the dpis, which will require more effort.
What happens if someone puts save_dir to $HOME?, should we restrict it maybe?
To give access to ~/, and yet still protect ~/.ssh, we could do something like this (simplified example):
unveil("/home/user", "rwc"); unveil("/home/user/.ssh", "");
But again, maybe it's better to override save_dir entirely if we are using unveil.
I think we could just not allow $HOME to be set as save_dir (or any directory that contains $HOME, like /home) and refuse to start if this is the case. Best, Rodrigo.