Bug 14241

Summary:	uudecode doesn't recognise the special decode_pathname /dev/stdout
Product:	Busybox	Reporter:	Christoph Anton Mitterer <calestyo>
Component:	Standard Compliance	Assignee:	unassigned
Status:	RESOLVED FIXED
Severity:	major	CC:	busybox-cvs, calestyo
Priority:	P5
Version:	1.30.x
Target Milestone:	---
Hardware:	All
OS:	All
Host:		Target:
Build:
Attachments:	uudecode-stdout-symbols-v1.patch uudecode-stdout-symbols-v2.patch

Description Christoph Anton Mitterer 2021-09-29 02:49:28 UTC

Hey.

uudecode can get the destination where output shall be written from two places:
a) the -o OUTFILE option
b) if (a) is not given, from the header of it's input data

For both, POSIX defines /dev/stdout to be a special symbol (rather than the real file) which shall lead uudecode to print the decoded data to its standard output.
See:
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/uudecode.html
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/uuencode.html

busybox' uudecode however, doesn't recognise that a special symbol, but simply takes it as a filename.

That will work, but only when it actually exists and in a way like on linux where it's a symbolic link to /proc/self/fd/1 .
However, there is no guarantee that it exists even there.

Could you please check, whether the decode_pathname (either via the header or -o) is /dev/stdout and if so, print to the real standard output of the process?

Maybe, but I'm not 100% sure about this, POSIX also specifies - to be such a special symbol.

It's a bit strange cause POSIX says for:
1) uudecode:
"If the file data header encoded by uuencode is - or /dev/stdout, or the -o /dev/stdout option overrides the file data, the standard output shall be in the same format as the file originally encoded by uuencode. Otherwise, the standard output shall not be used."

but at the same time says in the rationale, that - was specifically not standardised as meaning stdout:
"In early drafts, the [ -o outfile] option-argument allowed the use of - to mean standard output. The symbol - has only been used previously in POSIX.1-2017 as a standard input indicator. The standard developers did not wish to overload the meaning of - in this manner. The /dev/stdout concept exists on most modern systems. The /dev/stdout syntax does not refer to a new special file. It is just a magic cookie to specify standard output."

Cheers,
Chris.

Comment 1 Christoph Anton Mitterer 2021-09-29 03:10:36 UTC

I've just seen that GNU's uudecode is documented to take both /dev/stdout AND - as special symbols.

So I'd suggest that busybox does the same, regardless of what POSIX would say.

Comment 2 Christoph Anton Mitterer 2021-09-29 03:54:27 UTC

Created attachment 9096 [details]
uudecode-stdout-symbols-v1.patch

Comment 3 Christoph Anton Mitterer 2021-09-29 03:58:30 UTC

I've attached a patch, which should fix the issue (but someone with more knowledge on busybox should double check ;-) ).

Also it updates docs/posix_conformance.txt because uudecode clearly has -o, but I'm not sure whether it's POSIX-compliant or not (that would also require things like file permission bits restoration and so, I guess).


I've also increased severity, cause I think such a bug could at least potentially have security implications (imagine a user wants to make sure data is written to stdout and uses -o /dev/stdout for that) but in fact the data (which may be confidential) is written to a fresh file /dev/stdout, where everyone can read it.

Comment 4 Christoph Anton Mitterer 2021-09-29 04:02:15 UTC

Created attachment 9101 [details]
uudecode-stdout-symbols-v2.patch

Comment 5 Denys Vlasenko 2021-12-11 23:35:31 UTC

Fixed in git.

Comment 6 Christoph Anton Mitterer 2021-12-12 00:45:55 UTC

Any reason why you make this dependent on ENABLE_DESKTOP?

I mean POSIX is quite clear... it should simply always behave that way and especially not doing so can lead to security issues on systems where /dev/stdout doesn't exists as a file (which is linked to FD1).

Comment 7 Christoph Anton Mitterer 2021-12-12 00:53:00 UTC

I've reopened this... cause it seems ENABLE_DESKTOP is not enabled per default, and even if one chooses to enable it, several other things also change their behaviour.

So the problem is that the default setup of busybox' uudecode still behaves wrong and is thus vulnerable.

Comment 8 Christoph Anton Mitterer 2021-12-25 01:03:50 UTC

I should perhaps add one thing:

IMO it's okay to make features configurable, but only if the tool in question fails kinda gracefully if it was disabled.
"Gracefully" in the sense, that the failure is not expected by POSIX, but still detectable by checking the exit status

So for example:
if busybox' tr supports disabling character classes, fine, but only if tr then fails with some non-zero exit status whenever it encounters such character class.

The same would go here:
If busybox' uuencode was compiled without support for the symbol "/dev/stdout" it should then fail whnever that string is used.

Otherwise one cannot reliably use it, when one cannot know how it was compiled.

Comment 9 Denys Vlasenko 2021-12-30 16:59:26 UTC

> cause it seems ENABLE_DESKTOP is not enabled per default

ENABLE_DESKTOP is enabled per default.

I think this bug is closed.

Comment 10 Christoph Anton Mitterer 2022-04-10 22:26:27 UTC

Just for the records, if someone ever stumbles over this:

It seems as if "-" might also be standardised as a "special symbol" (just like the string /dev/stdout is) with the next revision of POSIX.
See https://www.austingroupbugs.net/view.php?id=1544 


The string "/dev/stdout" would however not be dropped from being required, too.