Opened 2 years ago

Closed 2 years ago

#18253 closed defect (fixed)

(Sandbox) Caught a bad syscall attempt (syscall chown)

Reported by: weasel Owned by: nickm
Priority: High Milestone: Tor: 0.2.8.x-final
Component: Core Tor/Tor Version: Tor: 0.2.8.1-alpha
Severity: Normal Keywords: must-fix-before-028-rc
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor: None

Description

On 0.2.8.1-alpha:

tor --defaults-torrc /usr/share/tor/tor-service-defaults-torrc -f /etc/tor/torrc --RunAsDaemon 0
tail -F /var/log/tor/log
[..]
Feb 05 23:15:14.000 [notice] Bootstrapped 0%: Starting
Feb 05 23:15:14.000 [notice] Bootstrapped 80%: Connecting to the Tor network
Feb 05 23:15:14.000 [notice] Bootstrapped 85%: Finishing handshake with first hop
Feb 05 23:15:15.000 [warn] sandbox_intern_string(): Bug: No interned sandbox parameter found for /var/run/tor (on Tor 0.2.8.1-alpha )
Feb 05 23:15:15.000 [notice] Opening Control listener on /var/run/tor/control

============================================================ T= 1454710515
(Sandbox) Caught a bad syscall attempt (syscall chown)
tor(+0x14ca96)[0x7f76431b4a96]
/lib/x86_64-linux-gnu/libc.so.6(chown+0x7)[0x7f764155bf07]
/lib/x86_64-linux-gnu/libc.so.6(chown+0x7)[0x7f764155bf07]
tor(retry_all_listeners+0x80e)[0x7f764314d14e]
tor(+0x3cf1d)[0x7f76430a4f1d]
tor(+0x56108)[0x7f76430be108]
tor(+0x3d34c)[0x7f76430a534c]
/usr/lib/x86_64-linux-gnu/libevent-2.0.so.5(+0x12584)[0x7f76426f3584]
/usr/lib/x86_64-linux-gnu/libevent-2.0.so.5(event_base_loop+0x7fc)[0x7f76426f13dc]
tor(do_main_loop+0x274)[0x7f76430a8d34]
tor(tor_main+0x1a55)[0x7f76430ac355]
tor(main+0x19)[0x7f76430a4979]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf5)[0x7f76414a0b45]
tor(+0x3c9c9)[0x7f76430a49c9]

This doesn't happen on 0.2.7.x

Child Tickets

Change History (12)

comment:1 Changed 2 years ago by nickm

Milestone: Tor: 0.2.8.x-final
Owner: set to nickm
Priority: MediumHigh
Status: newaccepted

comment:2 Changed 2 years ago by weasel

Bisecting between master and 0.2.7.6:

[git|dcbfe46...|bisect] weasel@valiant:~/projects/tor/tor$ git bisect bad 
dcbfe46cd63e041a5bfc4f1de008a7f9025dffce is the first bad commit
commit dcbfe46cd63e041a5bfc4f1de008a7f9025dffce
Author: Jamie Nguyen <j@jamielinux.com>
Date:   Fri Nov 13 13:57:11 2015 +0000

    Defer creation of Unix socket until after setuid

:040000 040000 676aa813efc517daf0b6100a11baba0729418bc7 0db72ad4c78b0a7aba8ec393e8744c187e966a18 M      changes
:040000 040000 1226793c73477f14743b846edd3bdc20440e8b43 1a06efee9841a5c1517004c2afd2e508a160caf6 M      src
[git|dcbfe46...|bisect] weasel@valiant:~/projects/tor/tor$ git bisect log
git bisect start
# bad: [1f5cdf2b6c72ae89eea630a2c4693273ff1aee6c] Merge branch 'maint-0.2.7'
git bisect bad 1f5cdf2b6c72ae89eea630a2c4693273ff1aee6c
# good: [9ccf019b168909ef6674c71aebda9f90516bb6a1] Merge branch 'maint-0.2.6' into release-0.2.6
git bisect good 9ccf019b168909ef6674c71aebda9f90516bb6a1
# good: [f620b8f03286de641329cd9ae2027f3628a7d2a1] bump version to 0.2.6.7-dev
git bisect good f620b8f03286de641329cd9ae2027f3628a7d2a1
# good: [047989ea288fa1e3099f1897a3596abd6f2e8860] fixup add malformed_list to unit tests from d3358a0a05f6 IPv6 wildcards
git bisect good 047989ea288fa1e3099f1897a3596abd6f2e8860
# good: [e5754c42d124549b3fd8e8d7c11d4dde3b5acec1] Merge branch 'bug17686_v2_027'
git bisect good e5754c42d124549b3fd8e8d7c11d4dde3b5acec1
# bad: [7b0cbf22c0fb347db1e7436464865d395aed0ce5] Merge remote-tracking branch 'yawning/feature17783_take2'
git bisect bad 7b0cbf22c0fb347db1e7436464865d395aed0ce5
# bad: [01334589f1eae801b4ed8fb72fe3816ad5b0fe78] Simplify cpd_opts usage.
git bisect bad 01334589f1eae801b4ed8fb72fe3816ad5b0fe78
# good: [252149e8b4697c570097acd514c433b8215f24dd] Merge branch 'maint-0.2.7'
git bisect good 252149e8b4697c570097acd514c433b8215f24dd
# good: [b3eba8ef12c714e1b97b9026da96d27433064a1e] Merge branch 'refactor-effective-entry'
git bisect good b3eba8ef12c714e1b97b9026da96d27433064a1e
# good: [390d3fa3af28825e152aadd244e3855d067179c0] add a static
git bisect good 390d3fa3af28825e152aadd244e3855d067179c0
# good: [9f6b9e28ccfdc3a96fb6e28d5121539f3cba3c55] forward-port changelog and releasenotes
git bisect good 9f6b9e28ccfdc3a96fb6e28d5121539f3cba3c55
# good: [d68b7fd4422f6ea1cad18a26b6a46b61bc182285] Refactor clock skew warning code to avoid duplication
git bisect good d68b7fd4422f6ea1cad18a26b6a46b61bc182285
# good: [ec4ef68271ab65b4ec643088153211e861cdc7b3] Introduce DataDirectoryGroupReadable boolean
git bisect good ec4ef68271ab65b4ec643088153211e861cdc7b3
# bad: [dcbfe46cd63e041a5bfc4f1de008a7f9025dffce] Defer creation of Unix socket until after setuid
git bisect bad dcbfe46cd63e041a5bfc4f1de008a7f9025dffce
# first bad commit: [dcbfe46cd63e041a5bfc4f1de008a7f9025dffce] Defer creation of Unix socket until after setuid

comment:3 Changed 2 years ago by weasel

If we remove the chown, arguing that the above match makes it no longer necessary [citation needed], we will then fall victim to the chmod a few lines down.

comment:4 Changed 2 years ago by nickm

Could we fix that with using fchown and fchmod?

comment:5 Changed 2 years ago by nickm

(Or do fchown and fchmod not work with sockets?)

comment:6 Changed 2 years ago by nickm

Keywords: must-fix-before-028-rc added

Marking these as must-fix-before-028-rc.

Actually, some of them may not need to get 'fixed' before the rc, but I believe that they should either get fixed, or we should have a good explanation of why they don't need to get fixed.

comment:7 Changed 2 years ago by nickm

Status: acceptedneeds_review

Branch bug18253 in my public repository fixes this issue for me. Please review?

comment:8 Changed 2 years ago by nickm

Sponsor: None

Weasel reports that somewhere in master, the behavior of this bug changed from a crash to a warning. He also reports that with my branch above, the warning goes away and stuff starts working.

That sounds like a fix to me. Can anybody have a look at the patch?

comment:9 Changed 2 years ago by Sebastian

Changes file says "Sandbox 1 enabled" which I think is an accident?

I like the rest of the patch, but I didn't follow the code to disentangle what not calling sandbox_add_addrinfo() means. The sandbox module is pretty underdocumented :/

comment:10 Changed 2 years ago by nickm

(The above was a review for #18548. "Sandbox 1 enabled" is intentional but mispunctuated.)

comment:11 Changed 2 years ago by nickm

Merging this one too.

comment:12 Changed 2 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed
Note: See TracTickets for help on using tickets.