[
https://issues.apache.org/jira/browse/CAMEL-26066?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18139161#comment-18139161
]
Claus Ibsen commented on CAMEL-26066:
-------------------------------------
_Claude Code on behalf of Claus Ibsen (davsclaus)_
[~fmariani] you are working in this area at the moment, so can you pick this
one up? Below are some notes on the code (main 3f0ea5674020), which your agent
can use as a starting point. A coding agent wrote them, so a human needs to
check them.
h3. Where
* {{ScpOperations.storeFile(name, exchange, size)}} (camel-jsch,
{{org.apache.camel.component.scp}}) turns {{name}} into the remote file with
{{getRemoteFile(name, cfg)}}, opens the exec channel and calls {{writeFile}}.
* {{writeFile}} splits the name on {{/}}. For each directory segment it sends
{{D0775 0 <dir>}} plus a line feed, and for the file {{C0<chmod> <count>
<filename>}} plus a line feed.
* The exec command itself ({{getScpCommand}}) only uses the configured
directory ({{getRemoteTarget}}), not the file name, so the protocol lines are
the only place the name goes.
h3. Suggested fix
* Validate the whole remote name at the start of {{storeFile}}, before
{{session.openChannel("exec")}}, so nothing is sent when the name is refused.
Checking the whole name covers the directory segments and the file name.
* Refuse any char where {{Character.isISOControl(c)}} is true (this covers
{{\n}}, {{\r}}, NUL and the rest of C0/C1), with a
{{GenericFileOperationFailedException}}. The message should say that the name
contains a control character, without echoing the raw name.
h3. Tests
* {{ScpSimpleProduceTest}} with {{ScpServerTestSupport}} (embedded Apache SSHD)
already covers the producer. Add cases with {{CamelFileName}} containing
{{\n}}, {{\r}} and {{\u0000}}, in the file name and in a directory segment (for
example {{"a\nb/file.txt"}}). Assert that the exchange fails with
{{GenericFileOperationFailedException}} and that no file is created on the
server side.
* Keep a positive case with a normal nested name, so the recursive {{D}}/{{E}}
path still works.
h3. Docs
* Add a short note in
{{components/camel-jsch/src/main/docs/scp-component.adoc}} that file names with
control characters are refused, and regenerate the catalog copy.
* Add an upgrade guide entry in {{camel-4x-upgrade-guide-4_24.adoc}}, because
such names failed differently before and now fail up front.
Fix Version is 4.24.0.
> camel-jsch - the scp producer should refuse a file name with a line break or
> another control character
> ------------------------------------------------------------------------------------------------------
>
> Key: CAMEL-26066
> URL: https://issues.apache.org/jira/browse/CAMEL-26066
> Project: Camel
> Issue Type: Improvement
> Components: camel-jsch
> Reporter: Claus Ibsen
> Priority: Major
> Fix For: 4.24.0
>
>
> The scp producer sends the name of the file it writes inside the scp
> protocol, on a line that ends with a line feed: {{D0775 0 <dir>}} for each
> directory in the name, then {{C0<chmod> <size> <name>}} for the file
> ({{ScpOperations.writeFile}}, main faa015319ee7, lines 458 and 488).
> The name comes from the {{CamelFileName}} header or the {{fileName}} option.
> The producer checks that it stays inside the starting directory, and the
> {{chmod}} option must be three octal digits, but nothing checks the name for
> a line feed or another control character, which file systems allow in a file
> name. A line feed would end the protocol line early, and the remote scp would
> read the rest of the name as the next line.
> *Proposal*:
> * refuse a name with a line feed, a carriage return or another control
> character with a {{GenericFileOperationFailedException}}, before anything is
> sent;
> * test it against the embedded SSH server of the camel-jsch tests;
> * mention it in the scp docs.
> _Claude Code on behalf of davsclaus_
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
